[PATCH] D112595: [IRSymTab] Mark __stack_chk_guard used

Wed Oct 27 18:46:54 PDT 2021

ychen added a comment.

In D112595#3090548 <https://reviews.llvm.org/D112595#3090548>, @tejohnson wrote:

> In D112595#3089374 <https://reviews.llvm.org/D112595#3089374>, @ychen wrote:
>
>> In D112595#3089372 <https://reviews.llvm.org/D112595#3089372>, @MaskRay wrote:
>>
>>> Can `__stack_chk_fail` be removed from `llvm/lib/Transforms/IPO/Internalize.cpp`?
>>
>> It would not be removed there. `Internalize.cpp` preserves both `__stack_chk_fail` and the guard symbol.
>
> I'm confused after reading the referenced bug, which is about doing something to be able to remove this value's hardcoding from Internalize.cpp.

Apology for the confusion. I was not clear. Looking at PR28061 again, I think this patch does *NOT* fix PR28061: they are related but separate issues. (patch summary updated)
UpdateCompilerUsed/Internalizer deals with builtin symbols preservation for regular LTO using legacy LTO API; "RuntimeLibcalls.def" deals with builtin function symbols preservation for the new LTO API, but not builtin global variable like `__stack_chk_guard`.

> Does the newly added test case fail without the IRSymtab change in this patch?

Yes, it does fail without this patch.

> There are a couple other symbols handled specially in Internalize.cpp (__stack_chk_fail, and __ssp_canary_word for AIX), should the all be handled uniformly?

__stack_chk_fail is listed in "RuntimeLibcalls.def". All symbols in "RuntimeLibcalls.def" is marked used already. Just added handling&test for `__ssp_canary_word`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112595/new/

https://reviews.llvm.org/D112595