Re: [Patch] MdePkg/BaseSafeIntLib: Add SafeIntLib class and instance

[Laszlo Ersek <lersek@redhat.com>]

On 02/13/18 18:15, Andrew Fish wrote:
> 
> 
>> On Feb 13, 2018, at 8:56 AM, Bret Barkelew
>> <Bret.Barkelew@microsoft.com <mailto:Bret.Barkelew@microsoft.com>> wrote:
>>
>> In response to the original question, I would content that our goal
>> should be "a". We should be allowing universal detection of errors
>> without the caller having to carry this detection code itself.
>>
>> The analog would be the safe string functions: if a buffer overflow
>> occurs, they don't find a way to "fix" the operation, but they
>> faithfully report an error.
>>
>> As such, I believe from my review that these functions work as intended.
>>
> 
> Bret,
> 
> I think Lazlo's point is that undefined behavior[1]  can cause the math
> function to break in the future and that we have to be very pedantic in
> how it is coded.

That's *exactly* my point, yes.

> Per the C standard it is legal for the compiler to
> optimized away undefined behavior[2], and clang is very aggressive about
> warning on undefined behavior and then updating the optimizer to remove
> the code in a future release.

Thank you for the independent confirmation :) (I'm reading and answering
your email after sending my previous one.)

> For example the BaseTool compression code
> broke with Xcode 9 recently due to the presence of an illegal 32-bit
> shift that was only hit when the optimizer inlined the function. While
> the compiler tries to emit warnings, or at least traps, for undefined
> behavior what we have seen with the Link Time Optimization is the code
> can just get removed. 

Huge kudos for this example!

> 
> [1] - Kind of clangs point of view on undefined behavior in
> C: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
> [2] - Example of undefined behavior in clang that emits a trap.
> Dereferencing NULL is undefined behavior in C so clang emits a trap, and
> optimizes way the code after the trap. 
> 
> ~/work/Compiler>cat undefined.c
> 
> int
> main ()
> {
>   int *Yikes = 0;
> 
>   *Yikes = 1;
>   return 0;
> }
> 
> ~/work/Compiler>clang -S -Os undefined.c
> ~/work/Compiler>cat undefined.S
> .section__TEXT,__text,regular,pure_instructions
> .macosx_version_min 10, 12
> .globl_main
> _main:                                  ## @main
> .cfi_startproc
> ## BB#0:
> pushq%rbp
> Lcfi0:
> .cfi_def_cfa_offset 16
> Lcfi1:
> .cfi_offset %rbp, -16
> movq%rsp, %rbp
> Lcfi2:
> .cfi_def_cfa_register %rbp
> ud2
> .cfi_endproc
> 
> 
> .subsections_via_symbols

and for this one.

Thank you!
Laszlo