This is an archive of the discontinued LLVM Phabricator instance.

[Diagnostics] Teach -Wnull-dereference about address_space attribute
ClosedPublic

Authored by xbolva00 on Oct 31 2019, 8:24 AM.

Details

Summary

Clang should not warn for:

test.c:2:12: warning: indirection of non-volatile null pointer will be deleted,

  not trap [-Wnull-dereference]
return *(int __attribute__((address_space(256))) *) 0;
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Solves PR42292.

Diff Detail

Event Timeline

xbolva00 created this revision.Oct 31 2019, 8:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2019, 8:24 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
xbolva00 marked an inline comment as done.Oct 31 2019, 8:33 AM
xbolva00 added inline comments.
clang/lib/Sema/SemaExpr.cpp
484–491

With this patch, we no longer warn for:

*(int attribute((address_space(0))) *) 0;

Worth to handle this case?

xbolva00 updated this revision to Diff 227286.Oct 31 2019, 8:36 AM

Added testcase with typedefed int.

xbolva00 edited the summary of this revision. (Show Details)Oct 31 2019, 8:44 AM
aaron.ballman accepted this revision.Nov 7 2019, 11:40 AM

LGTM with a testing request.

clang/lib/Sema/SemaExpr.cpp
484–491

I mildly think it would be useful to warn in that case, but I'm fine with that being done later.

clang/test/Sema/exprs.c
192

Can you add a test case for dereferencing for a read instead of a write, just to ensure we don't diagnose that case?

You may want to add the address_space(0) test case as well, with a FIXME that it should warn at some point.

This revision is now accepted and ready to land.Nov 7 2019, 11:40 AM
xbolva00 updated this revision to Diff 228298.Nov 7 2019, 1:23 PM

More tests.
Handle attribute((address_space(0))).

xbolva00 updated this revision to Diff 228301.Nov 7 2019, 1:31 PM

Removed unused variable.

xbolva00 updated this revision to Diff 228303.Nov 7 2019, 1:32 PM

Fixed comments.

Harbormaster completed remote builds in B40644: Diff 228303.
This revision was automatically updated to reflect the committed changes.

Hi. I have a suspicion this patch is causing a segfault for us when building zircon: https://ci.chromium.org/p/fuchsia/builders/ci/fuchsia-x64-debug-clang/b8897388724384628544

I'm almost done with a bisect and will provide a reproducer if this is the case. Just raising awareness in the meantime.

Yup, a bisect shows it was this patch that caused it. I filed a bug for tracking at https://bugs.llvm.org/show_bug.cgi?id=43950 and attached a reproducer. Could you look into this? Thanks.

clang/lib/Sema/SemaExpr.cpp
487

If I were to guess what would cause the segfault, I'd probably think it was the getPointeeType().getAddressSpace() in the event the type isn't actually a pointer.