This is an archive of the discontinued LLVM Phabricator instance.

Differential D51211

[Sema] Emit -Wformat properly for bitfield promotions.
ClosedPublic

Authored by ebevhan on Aug 24 2018, 4:27 AM.

Details

Reviewers
krememek
rsmith
aaron.ballman
Summary

The integer promotions apply to bitfields as well, but
rather oddly. If you have a bitfield with a lower width
than int, then the type of the member expression will
be int regardless of the type of the bitfield member.
This means that you can effectively get 'type demotion'
in a bitfield member expression.

However, when analyzing format string types, we
would look through argument promotions like integer
promotion. Looking through bitfield demotion means
that we would get the wrong type when analyzing,
hiding -Wformat issues.

This patch fixes this so that we only explicitly look
through integer and floating point promotion where
the result type is actually a promotion.

Diff Detail

Event Timeline

ebevhan created this revision.Aug 24 2018, 4:27 AM
Herald added a subscriber: cfe-commits. · View Herald TranscriptAug 24 2018, 4:27 AM
ebevhan added a comment.Sep 7 2018, 2:20 AM

Ping.

ebevhan added a comment.Sep 20 2018, 6:28 AM

Another ping. Anyone up for reviewing this patch?

ebevhan added a reviewer: aaron.ballman.Nov 30 2018, 6:18 AM
aaron.ballman added inline comments.Nov 30 2018, 6:40 AM
lib/Sema/SemaChecking.cpp
7713–7717

Can be simplified to: return ICE->getCastKind() == CK_IntegralCast && From->isPromotableIntegerType() && S.Context.getPromotedIntegerType(From) == To;

test/Sema/format-strings.c
699–700 ↗(On Diff #162340)

The test currently does not have a target triple, so this is likely to fail on some bots. Rather than tie this entire test to one architecture, I'd split the bit-field tests out into a separate file and pick an explicit triple that meets this assumption.

ebevhan updated this revision to Diff 176130.Nov 30 2018, 8:13 AM

Rebased and addressed review comments.

ebevhan marked 2 inline comments as done.Nov 30 2018, 8:14 AM
aaron.ballman added inline comments.Nov 30 2018, 9:44 AM
lib/Sema/SemaChecking.cpp
7711–7715

This check is not checking against the promoted type of the bit-field. See Sema::UsualArithmeticConversions() for an example of what I'm talking about. Is that expected?

test/Sema/format-strings-bitfield-promotion.c
1

Running your test through GCC looks like the behavior matches here for C; can you also add a C++ test that demonstrates the behavior does not change?

https://godbolt.org/z/zRYDMG

ebevhan marked an inline comment as done.Dec 3 2018, 5:24 AM
ebevhan added inline comments.
lib/Sema/SemaChecking.cpp
7711–7715

I'm not entirely sure what you mean. Are you referring to the type produced by isPromotableBitField? The source type of that promotion is what we don't want to see in these implicit casts.

We don't want to look through promotions here if we promoted from a type which was the result of a bitfield promotion, and that bitfield promotion was from a higher ranked type to a lower ranked type. so, if we have a bitfield of type short, then promoting that to int is fine, and we will give a warning for the short. But if the type of the bitfield is long, it could be promoted to int. However, the format specifier warning will look through these promotions and think that we passed an expression of long to the function even though it was int.

aaron.ballman added inline comments.Dec 3 2018, 6:23 AM
lib/Sema/SemaChecking.cpp
7711–7715

Ahhh, okay, I see what's going on then. This makes more sense to me now, thank you for the explanation.

test/Sema/format-strings-bitfield-promotion.c
1

Strangely, the above godbolt link dropped the output windows, here's a different link that shows the behavioral differences between C and C++ mode in GCC: https://godbolt.org/z/R3zRHe

ebevhan marked 2 inline comments as done.Dec 3 2018, 6:36 AM
ebevhan added inline comments.
test/Sema/format-strings-bitfield-promotion.c
1

Hmm, I'll have a look at this.

ebevhan marked an inline comment as done.Dec 3 2018, 6:57 AM
ebevhan added inline comments.
test/Sema/format-strings-bitfield-promotion.c
1

That gcc godbolt is a bit odd. The type of the bitfield expression in the C++ example is long and not int, but in Clang, it's clearly being converted. If I change the example a bit, we get this warning:

<source>:11:12: warning: format '%d' expects argument of type 'int', but argument 2 has type 'long int' [-Wformat=]
   11 |   printf("%d", bf.a); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
      |           ~^   ~~~~
      |            |      |
      |            int    long int

But in Clang, we get a cast to int:

| `-ImplicitCastExpr 0xd190748 <col:17, col:20> 'int' <IntegralCast>
|   `-ImplicitCastExpr 0xd190730 <col:17, col:20> 'long' <LValueToRValue>
|     `-MemberExpr 0xd190618 <col:17, col:20> 'long' lvalue bitfield .a 0xd18f790
|       `-DeclRefExpr 0xd1905f8 <col:17> 'struct bitfields':'bitfields' lvalue Var 0xd18fa18 'bf' 'struct bitfields':'bitfields'

So gcc and Clang are doing things differently here.

The code in isPromotableBitField says:

// FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
//        We perform that promotion here to match GCC and C++.

but clearly gcc isn't doing this in the C++ case. The comments also mention some things about gcc bugs that Clang does not follow, but that's in reference to a C DR.

aaron.ballman added inline comments.Dec 3 2018, 7:41 AM
test/Sema/format-strings-bitfield-promotion.c
1

C++ disallows the rank conversion from int to long as well. [conv.prom]p1 does not apply because long int has a higher rank than int, but [conv.prom]p5 allows the promotion if the range of values is identical between the two types.

C makes this UB in several ways -- you can't have a bit-field whose type is something other than int, unsigned int, or _Bool (6.7.2.1p5) or promoting from types other than those (6.3.1.1p2), but otherwise matches the C++ behavior in terms of promotion (including the rank conversion).

You may have to dig further into what Clang is doing, but I would guess that the diagnostics should be triggered in both C and C++ similarly.

Ultimately, I'd like to see tests for cases where sizeof(int) == sizeof(long), sizeof(int) != sizeof(long), and variants for C and C++ of each.

ebevhan added inline comments.Dec 4 2018, 2:10 AM
test/Sema/format-strings-bitfield-promotion.c
1

I'm not sure the warning should trigger in C++; the behavior is correct there. The expression in those cases should be of type long, not int. The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int, but the rules in C say that the value _is_ promoted.

The strange promotion behavior only occurs in C because of the issue with bitfields larger than int. It's not really permitted according to the standard, but it's supported anyway to match C++. Though, it ends up not matching C++ due to these promotion differences.

I'll add tests for the different int and long sizes, though the only case where it would make a difference would be if int was larger than 32 bits, which it isn't on any target.

ebevhan updated this revision to Diff 176571.Dec 4 2018, 2:18 AM

Added testing for C++ and different sizes of int and long.

aaron.ballman accepted this revision.Dec 4 2018, 2:16 PM

LGTM!

test/Sema/format-strings-bitfield-promotion.c
1

The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int, but the rules in C say that the value _is_ promoted.

Ahhh, that explains the differences (my eyes glossed over that in the standards text), thank you!

This revision is now accepted and ready to land.Dec 4 2018, 2:16 PM
ebevhan added a comment.Dec 11 2018, 8:02 AM

Thanks!

I don't have commit access, so I would appreciate it if you could commit the change.

aaron.ballman closed this revision.Dec 11 2018, 11:21 AM

I've commit in r348889, thank you for the patch!

aaron.ballman reopened this revision.Dec 11 2018, 11:46 AM

I had to revert the commit as the changes caused some test failures.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/40784/steps/test/logs/stdio

FAIL: Clang :: CodeGenOpenCL/printf.cl (8013 of 45545)
******************** TEST 'Clang :: CodeGenOpenCL/printf.cl' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=-+cl_khr_fp64 -triple spir-unknown-unknown -disable-llvm-passes -emit-llvm -o - /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl | /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/FileCheck -check-prefixes=FP64,ALL /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
: 'RUN: at line 2';   /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=-cl_khr_fp64 -triple spir-unknown-unknown -disable-llvm-passes -emit-llvm -o - /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl | /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/FileCheck -check-prefixes=NOFP64,ALL /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl
--
Exit Code: 2

Command Output (stderr):
--
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:20:18: warning: format specifies type 'double __attribute__((ext_vector_type(2)))' but the argument has type 'float2' (vector of 2 'float' values)
  printf("%v2f", arg);
          ~~~~   ^~~
          %v2f
clang: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/AST/ASTContext.cpp:5554: FloatingRank getFloatingRank(clang::QualType): Assertion `T->getAs<BuiltinType>() && "getFloatingRank(): not a floating type"' failed.
Stack dump:
0.	Program arguments: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=-cl_khr_fp64 -triple spir-unknown-unknown -disable-llvm-passes -emit-llvm -o - /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl 
1.	/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:30:21: current parser token ')'
2.	/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:29:42: parsing function body 'test_printf_half2'
3.	/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl:29:42: in compound statement ('{}')
#0 0x00000000015d9934 PrintStackTraceSignalHandler(void*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d9934)
#1 0x00000000015d769e llvm::sys::RunSignalHandlers() (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d769e)
#2 0x00000000015d9af8 SignalHandler(int) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d9af8)
#3 0x00007f95cde44890 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
#4 0x00007f95ccb0ae97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
#5 0x00007f95ccb0c801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
#6 0x00007f95ccafc39a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
#7 0x00007f95ccafc412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
#8 0x00000000032f872c getFloatingRank(clang::QualType) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x32f872c)
#9 0x00000000032f877e clang::ASTContext::getFloatingTypeOrder(clang::QualType, clang::QualType) const (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x32f877e)
#10 0x0000000002bf9316 (anonymous namespace)::CheckPrintfHandler::checkFormatExpr(clang::analyze_printf::PrintfSpecifier const&, char const*, unsigned int, clang::Expr const*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bf9316)
#11 0x0000000002bf39a1 (anonymous namespace)::CheckPrintfHandler::HandlePrintfSpecifier(clang::analyze_printf::PrintfSpecifier const&, char const*, unsigned int) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bf39a1)
#12 0x0000000003543641 clang::analyze_format_string::ParsePrintfString(clang::analyze_format_string::FormatStringHandler&, char const*, char const*, clang::LangOptions const&, clang::TargetInfo const&, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x3543641)
#13 0x0000000002bd8da2 checkFormatStringExpr(clang::Sema&, clang::Expr const*, llvm::ArrayRef<clang::Expr const*>, bool, unsigned int, unsigned int, clang::Sema::FormatStringType, clang::Sema::VariadicCallType, bool, llvm::SmallBitVector&, (anonymous namespace)::UncoveredArgHandler&, llvm::APSInt) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bd8da2)
#14 0x0000000002bd6dc2 clang::Sema::CheckFormatArguments(llvm::ArrayRef<clang::Expr const*>, bool, unsigned int, unsigned int, clang::Sema::FormatStringType, clang::Sema::VariadicCallType, clang::SourceLocation, clang::SourceRange, llvm::SmallBitVector&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bd6dc2)
#15 0x0000000002bcc193 clang::Sema::checkCall(clang::NamedDecl*, clang::FunctionProtoType const*, clang::Expr const*, llvm::ArrayRef<clang::Expr const*>, bool, clang::SourceLocation, clang::SourceRange, clang::Sema::VariadicCallType) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bcc193)
#16 0x0000000002bce123 clang::Sema::CheckFunctionCall(clang::FunctionDecl*, clang::CallExpr*, clang::FunctionProtoType const*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bce123)
#17 0x0000000002e20303 clang::Sema::BuildResolvedCallExpr(clang::Expr*, clang::NamedDecl*, clang::SourceLocation, llvm::ArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2e20303)
#18 0x0000000002e05563 clang::Sema::ActOnCallExpr(clang::Scope*, clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2e05563)
#19 0x0000000002956b9a clang::Parser::ParsePostfixExpressionSuffix(clang::ActionResult<clang::Expr*, true>) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2956b9a)
#20 0x0000000002958bb6 clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2958bb6)
#21 0x00000000029542a2 clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29542a2)
#22 0x00000000029541e9 clang::Parser::ParseExpression(clang::Parser::TypeCastState) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29541e9)
#23 0x000000000299c8a6 clang::Parser::ParseExprStatement() (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x299c8a6)
#24 0x000000000299b2e4 clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::AllowedConstructsKind, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x299b2e4)
#25 0x000000000299ada9 clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::AllowedConstructsKind, clang::SourceLocation*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x299ada9)
#26 0x00000000029a28ff clang::Parser::ParseCompoundStatementBody(bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29a28ff)
#27 0x00000000029a32f2 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29a32f2)
#28 0x0000000002915b55 clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2915b55)
#29 0x000000000292e3cd clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x292e3cd)
#30 0x0000000002914bea clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2914bea)
#31 0x00000000029146d3 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29146d3)
#32 0x0000000002912ff9 clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2912ff9)
#33 0x000000000291216b clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x291216b)
#34 0x000000000290da86 clang::ParseAST(clang::Sema&, bool, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x290da86)
#35 0x0000000001c7ed10 clang::FrontendAction::Execute() (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x1c7ed10)
#36 0x0000000001c249c1 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x1c249c1)
#37 0x0000000001d1af34 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x1d1af34)
#38 0x00000000008092d2 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x8092d2)
#39 0x0000000000807545 main (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x807545)
#40 0x00007f95ccaedb97 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b97)
#41 0x000000000080476a _start (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x80476a)
FileCheck error: '-' is empty.
FileCheck command line:  /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/FileCheck -check-prefixes=NOFP64,ALL /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/CodeGenOpenCL/printf.cl

--

********************
FAIL: Clang :: SemaOpenCL/printf-format-strings.cl (13929 of 45545)
******************** TEST 'Clang :: SemaOpenCL/printf-format-strings.cl' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=+cl_khr_fp64 -fsyntax-only -verify /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl
: 'RUN: at line 2';   /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=-cl_khr_fp64 -fsyntax-only -verify /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl
--
Exit Code: 134

Command Output (stderr):
--
clang: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/lib/AST/ASTContext.cpp:5554: FloatingRank getFloatingRank(clang::QualType): Assertion `T->getAs<BuiltinType>() && "getFloatingRank(): not a floating type"' failed.
Stack dump:
0.	Program arguments: /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=+cl_khr_fp64 -fsyntax-only -verify /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl 
1.	/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl:72:25: current parser token ')'
2.	/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl:71:1: parsing function body 'format_v4f32_wrong_num_elts'
3.	/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl:71:1: in compound statement ('{}')
#0 0x00000000015d9934 PrintStackTraceSignalHandler(void*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d9934)
#1 0x00000000015d769e llvm::sys::RunSignalHandlers() (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d769e)
#2 0x00000000015d9af8 SignalHandler(int) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x15d9af8)
#3 0x00007f5948744890 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
#4 0x00007f594740ae97 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
#5 0x00007f594740c801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
#6 0x00007f59473fc39a (/lib/x86_64-linux-gnu/libc.so.6+0x3039a)
#7 0x00007f59473fc412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
#8 0x00000000032f872c getFloatingRank(clang::QualType) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x32f872c)
#9 0x00000000032f877e clang::ASTContext::getFloatingTypeOrder(clang::QualType, clang::QualType) const (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x32f877e)
#10 0x0000000002bf9316 (anonymous namespace)::CheckPrintfHandler::checkFormatExpr(clang::analyze_printf::PrintfSpecifier const&, char const*, unsigned int, clang::Expr const*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bf9316)
#11 0x0000000002bf39a1 (anonymous namespace)::CheckPrintfHandler::HandlePrintfSpecifier(clang::analyze_printf::PrintfSpecifier const&, char const*, unsigned int) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bf39a1)
#12 0x0000000003543641 clang::analyze_format_string::ParsePrintfString(clang::analyze_format_string::FormatStringHandler&, char const*, char const*, clang::LangOptions const&, clang::TargetInfo const&, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x3543641)
#13 0x0000000002bd8da2 checkFormatStringExpr(clang::Sema&, clang::Expr const*, llvm::ArrayRef<clang::Expr const*>, bool, unsigned int, unsigned int, clang::Sema::FormatStringType, clang::Sema::VariadicCallType, bool, llvm::SmallBitVector&, (anonymous namespace)::UncoveredArgHandler&, llvm::APSInt) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bd8da2)
#14 0x0000000002bd6dc2 clang::Sema::CheckFormatArguments(llvm::ArrayRef<clang::Expr const*>, bool, unsigned int, unsigned int, clang::Sema::FormatStringType, clang::Sema::VariadicCallType, clang::SourceLocation, clang::SourceRange, llvm::SmallBitVector&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bd6dc2)
#15 0x0000000002bcc193 clang::Sema::checkCall(clang::NamedDecl*, clang::FunctionProtoType const*, clang::Expr const*, llvm::ArrayRef<clang::Expr const*>, bool, clang::SourceLocation, clang::SourceRange, clang::Sema::VariadicCallType) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bcc193)
#16 0x0000000002bce123 clang::Sema::CheckFunctionCall(clang::FunctionDecl*, clang::CallExpr*, clang::FunctionProtoType const*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2bce123)
#17 0x0000000002e20303 clang::Sema::BuildResolvedCallExpr(clang::Expr*, clang::NamedDecl*, clang::SourceLocation, llvm::ArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2e20303)
#18 0x0000000002e05563 clang::Sema::ActOnCallExpr(clang::Scope*, clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2e05563)
#19 0x0000000002956b9a clang::Parser::ParsePostfixExpressionSuffix(clang::ActionResult<clang::Expr*, true>) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2956b9a)
#20 0x0000000002958bb6 clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2958bb6)
#21 0x00000000029542a2 clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29542a2)
#22 0x00000000029541e9 clang::Parser::ParseExpression(clang::Parser::TypeCastState) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29541e9)
#23 0x000000000299c8a6 clang::Parser::ParseExprStatement() (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x299c8a6)
#24 0x000000000299b2e4 clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::AllowedConstructsKind, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x299b2e4)
#25 0x000000000299ada9 clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::AllowedConstructsKind, clang::SourceLocation*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x299ada9)
#26 0x00000000029a28ff clang::Parser::ParseCompoundStatementBody(bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29a28ff)
#27 0x00000000029a32f2 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29a32f2)
#28 0x0000000002915b55 clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2915b55)
#29 0x000000000292e3cd clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x292e3cd)
#30 0x0000000002914bea clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2914bea)
#31 0x00000000029146d3 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x29146d3)
#32 0x0000000002912ff9 clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x2912ff9)
#33 0x000000000291216b clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x291216b)
#34 0x000000000290da86 clang::ParseAST(clang::Sema&, bool, bool) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x290da86)
#35 0x0000000001c7ed10 clang::FrontendAction::Execute() (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x1c7ed10)
#36 0x0000000001c249c1 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x1c249c1)
#37 0x0000000001d1af34 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x1d1af34)
#38 0x00000000008092d2 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x8092d2)
#39 0x0000000000807545 main (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x807545)
#40 0x00007f59473edb97 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b97)
#41 0x000000000080476a _start (/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang+0x80476a)
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/SemaOpenCL/Output/printf-format-strings.cl.script: line 2: 63981 Aborted                 (core dumped) /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang -cc1 -internal-isystem /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0/include -nostdsysteminc -cl-std=CL1.2 -cl-ext=+cl_khr_fp64 -fsyntax-only -verify /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/SemaOpenCL/printf-format-strings.cl

--

********************
This revision is now accepted and ready to land.Dec 11 2018, 11:46 AM
ebevhan added a comment.Dec 12 2018, 12:23 AM

Hmm, sorry about that. I'll have a look.

ebevhan updated this revision to Diff 177831.Dec 12 2018, 2:39 AM

Fix the build failures (caused by default argument promotion of float vectors).

ebevhan requested review of this revision.Dec 12 2018, 7:08 AM
aaron.ballman added inline comments.Dec 14 2018, 12:44 PM
lib/Sema/SemaChecking.cpp
7713–7714

Looking at Sema::DefaultArgumentPromotion(), it seems there is some special logic there for _Float16 vs half/fp16. Do we need to deal with that here as well?

7715

Should use const auto * here and below.

ebevhan marked an inline comment as done.Dec 18 2018, 12:24 AM
ebevhan added inline comments.
lib/Sema/SemaChecking.cpp
7713–7714

I think it should be fine. If the special handling of those don't take, there won't be a default promotion for them, so this routine won't care about it.

ebevhan updated this revision to Diff 178605.Dec 18 2018, 12:27 AM

Use auto.

aaron.ballman accepted this revision.Dec 18 2018, 5:23 AM

LGTM then, thank you for the fix! Would you like me to commit this one for you?

This revision is now accepted and ready to land.Dec 18 2018, 5:23 AM
ebevhan added a comment.Dec 18 2018, 7:10 AM

Thanks, that would be great! Hopefully it will work this time.

aaron.ballman closed this revision.Dec 18 2018, 7:57 AM

I've commit as r349497, thank you!

Revision Contents

PathSize
lib/
Sema/
26 lines
test/
Sema/
17 lines

Diff 176130

lib/Sema/SemaChecking.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 7,691 Lines • ▼ Show 20 Lines else if (TrueTy.isNull())
return std::make_pair(FalseTy, FalseName); return std::make_pair(FalseTy, FalseName);
else if (FalseTy.isNull()) else if (FalseTy.isNull())
return std::make_pair(TrueTy, TrueName); return std::make_pair(TrueTy, TrueName);
} }
return std::make_pair(QualType(), StringRef()); return std::make_pair(QualType(), StringRef());
} }
/// Return true if \p ICE is an implicit argument promotion of an arithmetic
/// type. Bit-field 'promotions' from a higher ranked type to a lower ranked
/// type do not count.
static bool
isArithmeticArgumentPromotion(Sema &S, const ImplicitCastExpr *ICE) {
QualType From = ICE->getSubExpr()->getType();
QualType To = ICE->getType();
// It's a floating promotion if the source type is a lower rank.
if (ICE->getCastKind() == CK_FloatingCast &&
S.Context.getFloatingTypeOrder(From, To) < 0)
return true;
// It's an integer promotion if the destination type is the promoted
// source type.
return ICE->getCastKind() == CK_IntegralCast &&
From->isPromotableIntegerType() &&
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

Looking at Sema::DefaultArgumentPromotion(), it seems there is some special logic there for _Float16 vs half/fp16. Do we need to deal with that here as well?

aaron.ballman: Looking at `Sema::DefaultArgumentPromotion()`, it seems there is some special logic there for…
ebevhanAuthorUnsubmitted
Done
ReplyInline Actions

I think it should be fine. If the special handling of those don't take, there won't be a default promotion for them, so this routine won't care about it.

ebevhan: I think it should be fine. If the special handling of those don't take, there won't be a…
S.Context.getPromotedIntegerType(From) == To;
aaron.ballmanUnsubmitted
Done
ReplyInline Actions

This check is not checking against the promoted type of the bit-field. See Sema::UsualArithmeticConversions() for an example of what I'm talking about. Is that expected?

aaron.ballman: This check is not checking against the promoted type of the bit-field. See `Sema…
ebevhanAuthorUnsubmitted
Done
ReplyInline Actions

I'm not entirely sure what you mean. Are you referring to the type produced by isPromotableBitField? The source type of that promotion is what we don't want to see in these implicit casts.

We don't want to look through promotions here if we promoted from a type which was the result of a bitfield promotion, and that bitfield promotion was from a higher ranked type to a lower ranked type. so, if we have a bitfield of type short, then promoting that to int is fine, and we will give a warning for the short. But if the type of the bitfield is long, it could be promoted to int. However, the format specifier warning will look through these promotions and think that we passed an expression of long to the function even though it was int.

ebevhan: I'm not entirely sure what you mean. Are you referring to the type produced by…
aaron.ballmanUnsubmitted
Done
ReplyInline Actions

Ahhh, okay, I see what's going on then. This makes more sense to me now, thank you for the explanation.

aaron.ballman: Ahhh, okay, I see what's going on then. This makes more sense to me now, thank you for the…
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

Should use const auto * here and below.

aaron.ballman: Should use `const auto *` here and below.
}
aaron.ballmanUnsubmitted
Done
ReplyInline Actions

Can be simplified to: return ICE->getCastKind() == CK_IntegralCast && From->isPromotableIntegerType() && S.Context.getPromotedIntegerType(From) == To;

aaron.ballman: Can be simplified to: `return ICE->getCastKind() == CK_IntegralCast && From…
bool bool
CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
const char *StartSpecifier, const char *StartSpecifier,
unsigned SpecifierLen, unsigned SpecifierLen,
const Expr *E) { const Expr *E) {
using namespace analyze_format_string; using namespace analyze_format_string;
using namespace analyze_printf; using namespace analyze_printf;
Show All 11 LinesCheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
const analyze_printf::ArgType::MatchKind Match = const analyze_printf::ArgType::MatchKind Match =
AT.matchesType(S.Context, ExprTy); AT.matchesType(S.Context, ExprTy);
bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic; bool Pedantic = Match == analyze_printf::ArgType::NoMatchPedantic;
if (Match == analyze_printf::ArgType::Match) if (Match == analyze_printf::ArgType::Match)
return true; return true;
// Look through argument promotions for our error message's reported type. // Look through argument promotions for our error message's reported type.
// This includes the integral and floating promotions, but excludes array // This includes the integral and floating promotions, but excludes array
// and function pointer decay; seeing that an argument intended to be a // and function pointer decay (seeing that an argument intended to be a
// string has type 'char [6]' is probably more confusing than 'char *'. // string has type 'char [6]' is probably more confusing than 'char *') and
// certain bitfield promotions (bitfields can be 'demoted' to a lesser type).
if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) { if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
if (ICE->getCastKind() == CK_IntegralCast || if (isArithmeticArgumentPromotion(S, ICE)) {
ICE->getCastKind() == CK_FloatingCast) {
E = ICE->getSubExpr(); E = ICE->getSubExpr();
ExprTy = E->getType(); ExprTy = E->getType();
// Check if we didn't match because of an implicit cast from a 'char' // Check if we didn't match because of an implicit cast from a 'char'
// or 'short' to an 'int'. This is done because printf is a varargs // or 'short' to an 'int'. This is done because printf is a varargs
// function. // function.
if (ICE->getType() == S.Context.IntTy || if (ICE->getType() == S.Context.IntTy ||
ICE->getType() == S.Context.UnsignedIntTy) { ICE->getType() == S.Context.UnsignedIntTy) {
▲ Show 20 Lines • Show All 6,047 Lines • Show Last 20 Lines

test/Sema/format-strings-bitfield-promotion.c

  • This file was added.
// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

Running your test through GCC looks like the behavior matches here for C; can you also add a C++ test that demonstrates the behavior does not change?

https://godbolt.org/z/zRYDMG

aaron.ballman: Running your test through GCC looks like the behavior matches here for C; can you also add a…
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

Strangely, the above godbolt link dropped the output windows, here's a different link that shows the behavioral differences between C and C++ mode in GCC: https://godbolt.org/z/R3zRHe

aaron.ballman: Strangely, the above godbolt link dropped the output windows, here's a different link that…
ebevhanAuthorUnsubmitted
Not Done
ReplyInline Actions

Hmm, I'll have a look at this.

ebevhan: Hmm, I'll have a look at this.
ebevhanAuthorUnsubmitted
Done
ReplyInline Actions

That gcc godbolt is a bit odd. The type of the bitfield expression in the C++ example is long and not int, but in Clang, it's clearly being converted. If I change the example a bit, we get this warning:

<source>:11:12: warning: format '%d' expects argument of type 'int', but argument 2 has type 'long int' [-Wformat=]
   11 |   printf("%d", bf.a); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
      |           ~^   ~~~~
      |            |      |
      |            int    long int

But in Clang, we get a cast to int:

| `-ImplicitCastExpr 0xd190748 <col:17, col:20> 'int' <IntegralCast>
|   `-ImplicitCastExpr 0xd190730 <col:17, col:20> 'long' <LValueToRValue>
|     `-MemberExpr 0xd190618 <col:17, col:20> 'long' lvalue bitfield .a 0xd18f790
|       `-DeclRefExpr 0xd1905f8 <col:17> 'struct bitfields':'bitfields' lvalue Var 0xd18fa18 'bf' 'struct bitfields':'bitfields'

So gcc and Clang are doing things differently here.

The code in isPromotableBitField says:

// FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
//        We perform that promotion here to match GCC and C++.

but clearly gcc isn't doing this in the C++ case. The comments also mention some things about gcc bugs that Clang does not follow, but that's in reference to a C DR.

ebevhan: That gcc godbolt is a bit odd. The type of the bitfield expression in the C++ example is `long`…
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

C++ disallows the rank conversion from int to long as well. [conv.prom]p1 does not apply because long int has a higher rank than int, but [conv.prom]p5 allows the promotion if the range of values is identical between the two types.

C makes this UB in several ways -- you can't have a bit-field whose type is something other than int, unsigned int, or _Bool (6.7.2.1p5) or promoting from types other than those (6.3.1.1p2), but otherwise matches the C++ behavior in terms of promotion (including the rank conversion).

You may have to dig further into what Clang is doing, but I would guess that the diagnostics should be triggered in both C and C++ similarly.

Ultimately, I'd like to see tests for cases where sizeof(int) == sizeof(long), sizeof(int) != sizeof(long), and variants for C and C++ of each.

aaron.ballman: C++ disallows the rank conversion from int to long as well. [conv.prom]p1 does not apply…
ebevhanAuthorUnsubmitted
Not Done
ReplyInline Actions

I'm not sure the warning should trigger in C++; the behavior is correct there. The expression in those cases should be of type long, not int. The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int, but the rules in C say that the value _is_ promoted.

The strange promotion behavior only occurs in C because of the issue with bitfields larger than int. It's not really permitted according to the standard, but it's supported anyway to match C++. Though, it ends up not matching C++ due to these promotion differences.

I'll add tests for the different int and long sizes, though the only case where it would make a difference would be if int was larger than 32 bits, which it isn't on any target.

ebevhan: I'm not sure the warning should trigger in C++; the behavior is correct there. The expression…
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions

The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int, but the rules in C say that the value _is_ promoted.

Ahhh, that explains the differences (my eyes glossed over that in the standards text), thank you!

aaron.ballman: > The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int…
int printf(const char *restrict, ...);
struct bitfields {
long a : 2;
unsigned long b : 2;
long c : 32; // assumes that int is 32 bits
unsigned long d : 32; // assumes that int is 32 bits
} bf;
void bitfield_promotion() {
printf("%ld", bf.a); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
printf("%lu", bf.b); // expected-warning {{format specifies type 'unsigned long' but the argument has type 'int'}}
printf("%ld", bf.c); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
printf("%lu", bf.d); // expected-warning {{format specifies type 'unsigned long' but the argument has type 'unsigned int'}}
}