-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang] "modular_format" attribute for functions using format strings #147431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
6101f2a
906b0b4
4aaaa77
5ad6e9a
8d9246f
4f1d07c
ad4e425
ad78cf8
0afe987
afd92f6
56183a3
46f88d5
5e23447
25675a6
36b4e84
299c364
7272717
c067cce
a1c9f84
7cbf5b7
ffc8742
f951d6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9630,3 +9630,39 @@ silence diagnostics with code like: | |
| __attribute__((nonstring)) char NotAStr[3] = "foo"; // Not diagnosed | ||
| }]; | ||
| } | ||
|
|
||
| def ModularFormatDocs : Documentation { | ||
| let Category = DocCatFunction; | ||
| let Content = [{ | ||
| The ``modular_format`` attribute can be applied to a function that bears the | ||
| ``format`` attribute (or standard library functions) to indicate that the | ||
| implementation is "modular", that is, that the implementation is logically | ||
| divided into a number of named aspects. When the compiler can determine that | ||
| not all aspects of the implementation are needed for a given call, the compiler | ||
| may redirect the call to the identifier given as the first argument to the | ||
| attribute (the modular implementation function). | ||
|
|
||
| The second argument is a implementation name, and the remaining arguments are | ||
| aspects of the format string for the compiler to report. The implementation | ||
| name is an unevaluated identifier be in the C namespace. | ||
|
|
||
| The compiler reports that a call requires an aspect by issuing a relocation for | ||
| the symbol ``<impl_name>_<aspect>`` at the point of the call. This arranges for | ||
| code and data needed to support the aspect of the implementation to be brought | ||
| into the link to satisfy weak references in the modular implemenation function. | ||
| If the compiler does not understand an aspect, it must summarily consider any | ||
| call to require that aspect. | ||
|
|
||
| For example, say ``printf`` is annotated with | ||
| ``modular_format(__modular_printf, "__printf", "float")``. Then, a call to | ||
| ``printf(var, 42)`` would be untouched. A call to ``printf("%d", 42)`` would | ||
| become a call to ``__modular_printf`` with the same arguments, as would | ||
|
Comment on lines
+9658
to
+9659
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So will any call to Also, who is responsible for writing these attributes? Are they only in the libc implementation, or can a user write one of these themselves on their own declarations? I'm asking because I wonder about compatibility; e.g., the call dispatches to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's correct.
Users could use these for their own implementations, in particular to allow functions that e.g. wrap
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good to know, thanks!
My concern is more about dispatching in ways the user may not anticipate and getting observably different behavior. e.g., the user calls
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, if I understand what you're getting at, that can't happen: it's explicitly out of scope for the feature. The Accordingly, this feature would primarily be useful for cases where libc is statically linked in and paired with its own headers. (llvm-libc, various embedded libcs, etc.) I suppose it's technically possible to break out printf implementation parts into a family of individual dynamic libraries, but even then, any libc header set that required that the libc implementation be dynamically replaceable would not be able to include So, for implementations that use this feature, As an aside, this is my first time landing a RFC across so many components of LLVM. I wasn't sure how much detail to include in each change; my intuition was to try to provide links to the RFC instead. I don't want the above reasoning to get buried, and it gives me pause that it wasn't readily accessible. But I'm also not entirely sure where it should live going forward. Advice would be appreciated. |
||
| ``printf("%f", 42.0)``. The latter would be accompanied with a strong | ||
| relocation against the symbol ``__printf_float``, which would bring floating | ||
| point support for ``printf`` into the link. | ||
|
|
||
| The following aspects are currently supported: | ||
|
|
||
| - ``float``: The call has a floating point argument | ||
| }]; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2559,6 +2559,19 @@ void CodeGenModule::ConstructAttributeList(StringRef Name, | |
|
|
||
| if (TargetDecl->hasAttr<ArmLocallyStreamingAttr>()) | ||
| FuncAttrs.addAttribute("aarch64_pstate_sm_body"); | ||
|
|
||
| if (auto *ModularFormat = TargetDecl->getAttr<ModularFormatAttr>()) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Best I can tell, this is still only getting from ONE attribute. You probably have to do
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've made it so that attributes are merged together (trivially, allowing only duplicates), both across multiples per declaration and redeclarations, with the same semantics. That should allow getAttr, right?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| FormatAttr *Format = TargetDecl->getAttr<FormatAttr>(); | ||
| StringRef Type = Format->getType()->getName(); | ||
| std::string FormatIdx = std::to_string(Format->getFormatIdx()); | ||
| std::string FirstArg = std::to_string(Format->getFirstArg()); | ||
| SmallVector<StringRef> Args = { | ||
| Type, FormatIdx, FirstArg, | ||
| ModularFormat->getModularImplFn()->getName(), | ||
| ModularFormat->getImplName()}; | ||
| llvm::append_range(Args, ModularFormat->aspects()); | ||
| FuncAttrs.addAttribute("modular-format", llvm::join(Args, ",")); | ||
| } | ||
| } | ||
|
|
||
| // Attach "no-builtins" attributes to: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6973,6 +6973,65 @@ static void handleVTablePointerAuthentication(Sema &S, Decl *D, | |
| CustomDiscriminationValue)); | ||
| } | ||
|
|
||
| static bool modularFormatIsSame(const ModularFormatAttr *Existing, | ||
| IdentifierInfo *ModularImplFn, | ||
| StringRef ImplName, | ||
| ArrayRef<StringRef> Aspects) { | ||
| if (Existing->getModularImplFn() != ModularImplFn) | ||
| return false; | ||
| if (Existing->getImplName() != ImplName) | ||
| return false; | ||
| if (Existing->aspects_size() != Aspects.size()) | ||
| return false; | ||
| unsigned I = 0; | ||
| for (const auto &ExistingAspect : Existing->aspects()) { | ||
| if (ExistingAspect != Aspects[I++]) | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| static void handleModularFormat(Sema &S, Decl *D, const ParsedAttr &AL) { | ||
| StringRef ImplName; | ||
erichkeane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!S.checkStringLiteralArgumentAttr(AL, 1, ImplName)) | ||
| return; | ||
| SmallVector<StringRef> Aspects; | ||
| llvm::DenseSet<StringRef> SeenAspects; | ||
| bool HasDuplicate = false; | ||
| for (unsigned I = 2, E = AL.getNumArgs(); I != E; ++I) { | ||
| StringRef Aspect; | ||
| if (!S.checkStringLiteralArgumentAttr(AL, I, Aspect)) | ||
| return; | ||
| if (!SeenAspects.insert(Aspect).second) { | ||
| S.Diag(AL.getArgAsExpr(I)->getExprLoc(), | ||
| diag::err_modular_format_duplicate_aspect) | ||
| << Aspect; | ||
| HasDuplicate = true; | ||
| continue; | ||
| } | ||
| Aspects.push_back(Aspect); | ||
| } | ||
|
|
||
| // Store aspects sorted. | ||
| llvm::sort(Aspects); | ||
|
|
||
| IdentifierInfo *ModularImplFn = AL.getArgAsIdent(0)->getIdentifierInfo(); | ||
|
|
||
| if (const auto *Existing = D->getAttr<ModularFormatAttr>()) { | ||
| if (!modularFormatIsSame(Existing, ModularImplFn, ImplName, Aspects)) { | ||
| S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL; | ||
|
||
| S.Diag(Existing->getLocation(), diag::note_conflicting_attribute); | ||
| } | ||
| D->dropAttr<ModularFormatAttr>(); | ||
|
||
| } | ||
|
|
||
| if (HasDuplicate) | ||
|
||
| return; | ||
|
|
||
| D->addAttr(::new (S.Context) ModularFormatAttr( | ||
| S.Context, AL, ModularImplFn, ImplName, Aspects.data(), Aspects.size())); | ||
| } | ||
|
|
||
| //===----------------------------------------------------------------------===// | ||
| // Top Level Sema Entry Points | ||
| //===----------------------------------------------------------------------===// | ||
|
|
@@ -7910,6 +7969,10 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, | |
| case ParsedAttr::AT_VTablePointerAuthentication: | ||
| handleVTablePointerAuthentication(S, D, AL); | ||
| break; | ||
|
|
||
| case ParsedAttr::AT_ModularFormat: | ||
| handleModularFormat(S, D, AL); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s | ||
|
|
||
| int printf(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"))); | ||
| int myprintf(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"), format(printf, 1, 2))); | ||
erichkeane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // CHECK-LABEL: define dso_local void @test_inferred_format( | ||
| // CHECK: {{.*}} = call i32 (ptr, ...) @printf(ptr noundef @.str) #[[ATTR:[0-9]+]] | ||
| void test_inferred_format(void) { | ||
| printf("hello"); | ||
| } | ||
|
|
||
| // CHECK-LABEL: define dso_local void @test_explicit_format( | ||
| // CHECK: {{.*}} = call i32 (ptr, ...) @myprintf(ptr noundef @.str) #[[ATTR:[0-9]+]] | ||
| void test_explicit_format(void) { | ||
| myprintf("hello"); | ||
| } | ||
|
|
||
| int redecl(const char *fmt, ...) __attribute__((modular_format(__first_impl, "__first", "one"), format(printf, 1, 2))); | ||
| int redecl(const char *fmt, ...) __attribute__((modular_format(__second_impl, "__second", "two", "three"))); | ||
|
|
||
| // CHECK-LABEL: define dso_local void @test_redecl( | ||
| // CHECK: {{.*}} = call i32 (ptr, ...) @redecl(ptr noundef @.str) #[[ATTR_REDECL:[0-9]+]] | ||
| void test_redecl(void) { | ||
| redecl("hello"); | ||
| } | ||
|
|
||
| int order1(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "a", "b"), format(printf, 1, 2))); | ||
| int order2(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "b", "a"), format(printf, 1, 2))); | ||
|
|
||
| // CHECK-LABEL: define dso_local void @test_order( | ||
| // CHECK: {{.*}} = call i32 (ptr, ...) @order1(ptr noundef @.str) #[[ATTR_ORDER:[0-9]+]] | ||
| // CHECK: {{.*}} = call i32 (ptr, ...) @order2(ptr noundef @.str) #[[ATTR_ORDER]] | ||
| void test_order(void) { | ||
| order1("hello"); | ||
| order2("hello"); | ||
| } | ||
|
|
||
| int overwrite(const char *fmt, ...) __attribute__((modular_format(__impl1, "__name1", "1"), modular_format(__impl2, "__name2", "2"), format(printf, 1, 2))); | ||
|
|
||
| // CHECK-LABEL: define dso_local void @test_overwrite( | ||
| // CHECK: {{.*}} = call i32 (ptr, ...) @overwrite(ptr noundef @.str) #[[ATTR_OVERWRITE:[0-9]+]] | ||
| void test_overwrite(void) { | ||
| overwrite("hello"); | ||
| } | ||
|
|
||
| // CHECK: attributes #[[ATTR]] = { "modular-format"="printf,1,2,__modular_printf,__printf,float" } | ||
| // CHECK: attributes #[[ATTR_REDECL]] = { "modular-format"="printf,1,2,__second_impl,__second,three,two" } | ||
| // CHECK: attributes #[[ATTR_ORDER]] = { "modular-format"="printf,1,2,__modular_printf,__printf,a,b" } | ||
| // CHECK: attributes #[[ATTR_OVERWRITE]] = { "modular-format"="printf,1,2,__impl2,__name2,2" } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| //RUN: %clang_cc1 -fsyntax-only -verify %s | ||
|
|
||
| int printf(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"))); // no-error | ||
| int myprintf(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"))); // expected-error {{'modular_format' attribute requires 'format' attribute}} | ||
|
|
||
| int dupe(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float", "int", "float"), format(printf, 1, 2))); // expected-error {{duplicate aspect 'float' in 'modular_format' attribute}} | ||
| int multi_dupe(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float", "int", "float", "int"), format(printf, 1, 2))); // expected-error {{duplicate aspect 'float' in 'modular_format' attribute}} \ | ||
| // expected-error {{duplicate aspect 'int' in 'modular_format' attribute}} | ||
|
|
||
| // Test with multiple identical attributes on the same declaration. | ||
| int same_attr(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"), modular_format(__modular_printf, "__printf", "float"), format(printf, 1, 2))); // no-warning | ||
|
|
||
| // Test with multiple different attributes on the same declaration. | ||
| int diff_attr(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"), format(printf, 1, 2), modular_format(__modular_printf, "__printf", "int"))); // expected-warning {{attribute 'modular_format' is already applied with different arguments}} expected-note {{conflicting attribute is here}} | ||
|
|
||
| int diff_attr2(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"), format(printf, 1, 2), modular_format(__modular_printf, "__other", "float"))); // expected-warning {{attribute 'modular_format' is already applied with different arguments}} expected-note {{conflicting attribute is here}} | ||
|
|
||
| int diff_attr3(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float"), format(printf, 1, 2), modular_format(__other, "__printf", "float"))); // expected-warning {{attribute 'modular_format' is already applied with different arguments}} expected-note {{conflicting attribute is here}} | ||
|
|
||
| // Test with same attributes but different aspect order. | ||
| int diff_order(const char *fmt, ...) __attribute__((modular_format(__modular_printf, "__printf", "float", "int"), format(printf, 1, 2), modular_format(__modular_printf, "__printf", "int", "float"))); // no-warning |
Uh oh!
There was an error while loading. Please reload this page.