This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Workaround crash on encountering Class non-type template parameters
ClosedPublic

Authored by steakhal on Oct 12 2022, 4:35 AM.

Details

Summary

The Clang Static Analyzer will crash on this code:

struct Box {
  int value;
};
template <Box V> int get() {
  return V.value;
}
template int get<Box{-1}>();

https://godbolt.org/z/5Yb1sMMMb

The problem is that we don't account for encountering TemplateParamObjectDecls
within the DeclRefExpr handler in the ExprEngine.

IMO we should create a new memregion for representing such template
param objects, to model their language semantics.
Such as:

  • it should have global static storage
  • for two identical values, their addresses should be identical as well

http://eel.is/c%2B%2Bdraft/temp.param#8

I was thinking of introducing a TemplateParamObjectRegion under DeclRegion
for this purpose. It could have TemplateParamObjectDecl as a field.

The TemplateParamObjectDecl::getValue() returns APValue, which might
represent multiple levels of structures, unions and other goodies -
making the transformation from APValue to SVal a bit complicated.

That being said, for now, I think having Unknowns for such cases is
definitely an improvement to crashing, hence I'm proposing this patch.

Diff Detail

Event Timeline

steakhal created this revision.Oct 12 2022, 4:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2022, 4:35 AM
steakhal requested review of this revision.Oct 12 2022, 4:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2022, 4:35 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
xazax.hun accepted this revision.Oct 12 2022, 1:19 PM

This change look good to me. I am not entirely sure about the future plans though. We should be able to read the values out from the AST, so we do not need the store to map memregions to values (as far as the values inside the object are concerned). How to deal with the address of such object is a good question, but a special symbol or constant might be an alternative to introducing memregions.

This revision is now accepted and ready to land.Oct 12 2022, 1:19 PM

This change look good to me. I am not entirely sure about the future plans though.

Me neither. That was just a thought.

We should be able to read the values out from the AST, so we do not need the store to map memregions to values (as far as the values inside the object are concerned).

That's true.

How to deal with the address of such object is a good question, but a special symbol or constant might be an alternative to introducing memregions.

I'm not sure if we should introduce more and more special cases.
We should experiment with some ideas, that's for sure.