This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] IHexELFBuilder::addDataSections - fix evaluation ordering static analyzer warning
ClosedPublic

Authored by RKSimon on Aug 2 2021, 6:34 AM.

Details

Summary

As detailed on https://pvs-studio.com/en/blog/posts/cpp/0771/ and raised on D62583, the SecNo++increment is not guaranteed to occur before the second use of SecNo in the same addSection() call.

This patch pulls out the increment and adjusts the second use of SecNo accordingly.

Diff Detail

Event Timeline

RKSimon created this revision.Aug 2 2021, 6:34 AM
RKSimon requested review of this revision.Aug 2 2021, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2021, 6:34 AM
Herald added a subscriber: MaskRay. · View Herald Transcript
ikudrin added a subscriber: ikudrin.Aug 2 2021, 7:33 AM
ikudrin added inline comments.
llvm/tools/llvm-objcopy/ELF/Object.cpp
1345

This change is irrelevant to the issue.

1352

Object::sortSections() uses llvm::stable_sort(), thus, you may just pass a constant, e.g. 0, as the last argument.

RKSimon updated this revision to Diff 363532.Aug 2 2021, 11:27 AM
ikudrin accepted this revision.Aug 2 2021, 10:58 PM
ikudrin added inline comments.
llvm/tools/llvm-objcopy/ELF/Object.cpp
1348

Please add a note that passing a constant is allowed because sections are sorted with stable_sort().

1353

It looks like there is no more need to extract the increment as a separate statement because SecNo is now used only once in the expression. But if you find it more readable (and I do), please keep it as is.

This revision is now accepted and ready to land.Aug 2 2021, 10:58 PM
This revision was landed with ongoing or failed builds.Aug 3 2021, 4:28 AM
This revision was automatically updated to reflect the committed changes.