Page MenuHomePhabricator

[GlobalValue] Make dso_local function work with comdat nodeduplicate
ClosedPublic

Authored by MaskRay on Jan 12 2022, 11:54 PM.

Details

Summary

This fixes -fno-semantic-interposition -fsanitize-coverage incompatibility.

-fPIC -fno-semantic-interposition may add dso_local to an external linkage
function. -fsanitize-coverage instrumentation does not clear dso_local when
adding comdat nodeduplicate. This causes a compatibility issue: the function
symbol may be referenced by a PC-relative relocation without using the local
alias. In -shared mode, ld will report a relocation error.

The fix is to either clear dso_local when adding comdat nodeduplicate, or
supporting comdat nodeduplicate. The latter is more appropriate, because a
comdat nodeduplicate is like not using comdat.

Note: The comdat condition was originally added by D77429 to not use local alias
for a hidden external linkage function in a deduplicate comdat. The condition
has been unused since the code was refactored to only use local alias for
default visibility symbols.
Note: canBenefitFromLocalAlias is used by clang/lib/CodeGen/CodeGenModule.cpp
and we don't want to add dso_local to default visibility external linkage comdat any
(clang/test/CodeGenCUDA/usual-deallocators.cu).

Diff Detail

Event Timeline

MaskRay created this revision.Jan 12 2022, 11:54 PM
MaskRay requested review of this revision.Jan 12 2022, 11:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2022, 11:54 PM

I had to look into the documentation to understand how comdat works. Compared to the diffs I generated early this seems fine. I also did a full bootstrap multi-stage build of clang with this patch and the patches and flags used by my host clang. I can't reproduce the issue I described on D102453, so it seems fixed. Thanks!

ljmf00 accepted this revision.Jan 13 2022, 3:09 PM
This revision is now accepted and ready to land.Jan 13 2022, 3:09 PM

Chatted with @leonardchan: let me just submit it.

MaskRay updated this revision to Diff 399834.Jan 13 2022, 4:32 PM
MaskRay edited the summary of this revision. (Show Details)

Fix CodeGenCUDA/usual-deallocators.cu (strange default visibility external linkage comdat)

This revision was landed with ongoing or failed builds.Jan 13 2022, 4:37 PM
This revision was automatically updated to reflect the committed changes.