Page MenuHomePhabricator

[dfsan] Rename and fix an internal test issue for mmap+calloc
ClosedPublic

Authored by stephan.yichao.zhao on Apr 30 2021, 10:23 AM.

Details

Summary

The linker suggests using -Wl,-z,notext.

Replaced assert by exit also fixed this.

After renaming, interceptor.c would be used to test interceptors in general by D101204.

Diff Detail

Event Timeline

stephan.yichao.zhao requested review of this revision.Apr 30 2021, 10:23 AM
stephan.yichao.zhao created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2021, 10:23 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
morehouse added inline comments.May 3 2021, 10:48 AM
compiler-rt/test/dfsan/mmap_at_init.c
2

This test-only change is fine with me, but why do we need it in the first place? What relocation do we have marked read-only?

replaced assert by exit.

compiler-rt/test/dfsan/mmap_at_init.c
2

Not sure exactly which part causes the issue. It relates to assert: line 24 is assert.

ld.lld: error: can't create dynamic relocation R_X86_64_64 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
>>> defined in .../interceptors.c.tmp-calloc.o
>>> referenced by interceptors.c:24 (.../compiler-rt/test/dfsan/interceptors.c:24)
>>>               .../interceptors.c.tmp-calloc.o:(calloc)

Changed assert to an exit. Then it works.

morehouse added inline comments.
compiler-rt/test/dfsan/mmap_at_init.c
2

This is odd. @pcc, @eugenis: any idea why we get this linker error?

morehouse accepted this revision.May 6 2021, 8:46 AM

Change is fine with me, though I wish we understood why it's needed.

This revision is now accepted and ready to land.May 6 2021, 8:46 AM
stephan.yichao.zhao edited the summary of this revision. (Show Details)May 6 2021, 5:54 PM

Change is fine with me, though I wish we understood why it's needed.

still not sure why assert can cause link errors yet...
I added some comments in the test for reference.

This revision was landed with ongoing or failed builds.May 6 2021, 5:58 PM
This revision was automatically updated to reflect the committed changes.