This is an archive of the discontinued LLVM Phabricator instance.

[Analyzer] GNU named variadic macros in Plister
ClosedPublic

Authored by chrish_ericsson_atx on Sep 18 2020, 2:12 PM.

Details

Summary

Added support for GNU named variadic macros in
macro expansion for plist generation.

Fix for https://bugs.llvm.org/show_bug.cgi?id=44493

Diff Detail

Event Timeline

chrish_ericsson_atx requested review of this revision.Sep 18 2020, 2:12 PM
Ka-Ka added a subscriber: Ka-Ka.Sep 18 2020, 11:10 PM
clang/test/Analysis/plist-macros-with-expansion.cpp
536

Seems the do {} while (0) idiom is a problem, because the plist output gets formatted differently on windows than it does on Linux. (Extra space).

The fix is great, thank you so much! The test seems to be annoying, it might be worth looking into it some other time, but that is definitely orthogonal to this patch.

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
1211–1213

That looks so much better.

clang/test/Analysis/plist-macros-with-expansion.cpp
529–545

These FileCheck lines are great, because the plist file on its own is not very readable, could you include them for the new test as well? Side note, we're already thinking that removing the plist file might be a good idea (its a mess to keep up-to-date, and doesn't test all that much).

536

Oh that is a real shame, we absolutely shouldn't do that! Can we somehow provoke a warning through some other way? (null pointer dereference)

Addressed feedback from @Szelethus

chrish_ericsson_atx marked 2 inline comments as done.Sep 21 2020, 6:30 AM

Thanks for the quick feedback, Kristof! I changed the macro to be just a decrement operator, and triggered DBZ with that. Added the checks as you suggested.

Szelethus accepted this revision.Sep 21 2020, 11:41 AM

LGTM! Thanks!

This revision is now accepted and ready to land.Sep 21 2020, 11:41 AM
This revision was landed with ongoing or failed builds.Sep 21 2020, 1:39 PM
This revision was automatically updated to reflect the committed changes.