zahiraam added a comment. In D151834#4644394 <https://reviews.llvm.org/D151834#4644394>, @aaron.ballman wrote:
> In D151834#4644391 <https://reviews.llvm.org/D151834#4644391>, @zahiraam > wrote: > >> In D151834#4644378 <https://reviews.llvm.org/D151834#4644378>, >> @aaron.ballman wrote: >> >>> In D151834#4644375 <https://reviews.llvm.org/D151834#4644375>, @zahiraam >>> wrote: >>> >>>> In D151834#4644373 <https://reviews.llvm.org/D151834#4644373>, >>>> @aaron.ballman wrote: >>>> >>>>> In D151834#4643925 <https://reviews.llvm.org/D151834#4643925>, @uabelho >>>>> wrote: >>>>> >>>>>> Hi @zahiraam , >>>>>> >>>>>> I have a couple of downstream testcases that fail with this patch. >>>>>> Before >>>>>> >>>>>> > clang bbi-86364.c -lm -O3 >>>>>> > ./a.out >>>>>> >>>>>> passed but with the patch the assert in the program fails: >>>>>> >>>>>> a.out: bbi-86364.c:9: int main(): Assertion `(*__errno_location ()) == >>>>>> 33' failed. >>>>>> >>>>>> Is this as expected? >>>>>> >>>>>> F29200339: bbi-86364.c <https://reviews.llvm.org/F29200339> >>>>> >>>>> This seems unexpected to me and it seems to relate to whether you include >>>>> errno.h or not: https://godbolt.org/z/EPWzazx9r -- @zahiraam do you have >>>>> ideas as to what's going on? >>>> >>>> I haven't looked at it as I saw that the comment has been deleted. Let me >>>> look into it. >>> >>> Oh, it's not the inclusion of errno.h that matters, it's the declaration of >>> `__errno_location`: https://godbolt.org/z/zo4PaPEME -- it seems that >>> inclusion of `__attribute__ ((__const__))` is what makes the distinction: >>> https://godbolt.org/z/1bePhvaG4 >> >> Yes confirming this. In the errno.h header the function is declared: extern >> int *__errno_location (void) __attribute__ ((__nothrow__ )) __attribute__ >> ((__const__)); >> Since we can't really change the header, this needs to be changed in the >> compiler? > > Yeah, I think this is a bug with your patch, but I'm not certain where. I > suspect the changes to CGBuiltin.cpp are what are causing this issue -- CC > @efriedma and @rjmccall as they might have ideas. I think the issue comes from here: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L2365 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151834/new/ https://reviews.llvm.org/D151834 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
