This is an archive of the discontinued LLVM Phabricator instance.

[clang] Do not crash on pointer wchar_t pointer assignment.
ClosedPublic

Authored by adamcz on Nov 17 2020, 5:27 AM.

Details

Summary

wchar_t can be signed (thus hasSignedIntegerRepresentation() returns
true), but it doesn't have an unsigned type, which would lead to a crash
when trying to get it.

With this fix, we special-case WideChar types in the pointer assignment
code.

Diff Detail

Event Timeline

adamcz created this revision.Nov 17 2020, 5:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 17 2020, 5:27 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
adamcz requested review of this revision.Nov 17 2020, 5:27 AM
adamcz updated this revision to Diff 305752.

rebase

hokein added inline comments.Nov 17 2020, 12:13 PM
clang/lib/Sema/SemaExpr.cpp
8726 ↗(On Diff #305752)

I'm wondering whether the fix is in ASTContext::getCorrespondingUnsignedType()

if my reading of the standard is right:

I think the rule also applies on wchar_t, the fix seems to handle WChar_S case (return ASTContext::getUnsignedWCharType()) in ASTContext::getCorrespondingUnsignedType(). What do you think?

adamcz updated this revision to Diff 306108.Nov 18 2020, 7:53 AM

addressed review comment

clang/lib/Sema/SemaExpr.cpp
8726 ↗(On Diff #305752)

http://eel.is/c++draft/basic.fundamental#11 - wchar_t is not a signed or unsigned integer type. It is an integer type, but not signed integer type. Therefore it's underlying type might have an matching signed/unsigned version, but wchar_t does not.

I initially wanted to fix ASTContext::getCorrespondingUnsignedType(), but switched to this version with the idea that calling getCorrespondingUnsignedType() on wchar_t is most likely a bug in the first place and returning something there might just hide the issue.

However, based on your comment and the fact that, for char, we already return underlying type too, I think maybe this is a better approach after all. Updated. Thanks!

hokein accepted this revision.Nov 18 2020, 11:40 AM

Thanks!

This revision is now accepted and ready to land.Nov 18 2020, 11:40 AM
This revision was landed with ongoing or failed builds.Nov 20 2020, 6:33 AM
This revision was automatically updated to reflect the committed changes.