This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Do not segfault when doing logical and/or operations on symbols that have no output sections.
ClosedPublic

Authored by grimar on Aug 1 2017, 5:57 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Aug 1 2017, 5:57 AM
grimar updated this revision to Diff 109291.Aug 2 2017, 1:09 AM
  • Addressed review comment.
ruiu added inline comments.Aug 16 2017, 3:46 PM
test/ELF/linkerscript/early-assign-symbol.s
10–15 ↗(On Diff #109291)

What is the difference between these two tests? If they are testing the same thing, remove one of them. Unnecessary tests do harm than good. Please do not add tests for "just in case".

grimar added inline comments.Aug 17 2017, 12:29 AM
test/ELF/linkerscript/early-assign-symbol.s
10–15 ↗(On Diff #109291)

First checks | and second checks & operator.

Them are implemented via bitAnd() and bitOr() (both use getValue() changed in patch atm),
so I think we want to check both flows ?

ruiu added inline comments.Aug 17 2017, 8:20 AM
test/ELF/linkerscript/early-assign-symbol.s
10–15 ↗(On Diff #109291)

No, as long as one test covers new code, adding more test is mostly redundant. Just like attempting new code concise and clean, you want to try to make tests concise.

grimar updated this revision to Diff 111624.Aug 18 2017, 1:32 AM
  • Addressed review comment.
ruiu accepted this revision.Aug 18 2017, 2:35 PM

LGTM

This revision is now accepted and ready to land.Aug 18 2017, 2:35 PM
This revision was automatically updated to reflect the committed changes.