https://github.com/IamYJLee updated https://github.com/llvm/llvm-project/pull/201481
>From 71c849ed2b0e57d9bfcff0bb2d3900e4a508e1b3 Mon Sep 17 00:00:00 2001 From: LeeYoungJoon <[email protected]> Date: Thu, 4 Jun 2026 09:14:15 +0900 Subject: [PATCH 1/5] [clang][AST] Fix StmtProfile handling of GCCAsmStmt asm strings and clobbers did not profile asm strings and clobbers because they are not child statements. As a result, different inline asm statements could produce the same profile. This fixes a false positive in where branches containing inline asm were incorrectly reported as identical. --- clang/lib/AST/StmtProfile.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp index 22e2cc56bc700..90eab530e0c2e 100644 --- a/clang/lib/AST/StmtProfile.cpp +++ b/clang/lib/AST/StmtProfile.cpp @@ -332,7 +332,7 @@ void StmtProfiler::VisitGCCAsmStmt(const GCCAsmStmt *S) { VisitStmt(S); ID.AddBoolean(S->isVolatile()); ID.AddBoolean(S->isSimple()); - VisitExpr(S->getAsmStringExpr()); + Visit(S->getAsmStringExpr()); ID.AddInteger(S->getNumOutputs()); for (unsigned I = 0, N = S->getNumOutputs(); I != N; ++I) { ID.AddString(S->getOutputName(I)); @@ -345,7 +345,7 @@ void StmtProfiler::VisitGCCAsmStmt(const GCCAsmStmt *S) { } ID.AddInteger(S->getNumClobbers()); for (unsigned I = 0, N = S->getNumClobbers(); I != N; ++I) - VisitExpr(S->getClobberExpr(I)); + Visit(S->getClobberExpr(I)); ID.AddInteger(S->getNumLabels()); for (auto *L : S->labels()) VisitDecl(L->getLabel()); >From 72f49200bb459c4e0ba17a828d203e96e69a77ab Mon Sep 17 00:00:00 2001 From: LeeYoungJoon <[email protected]> Date: Thu, 4 Jun 2026 16:58:21 +0900 Subject: [PATCH 2/5] Add test case for branch clone inline asm --- .../bugprone/branch-clone-inline-asm.cpp | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp new file mode 100644 index 0000000000000..05027fb18b5ab --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp @@ -0,0 +1,75 @@ +// RUN: %check_clang_tidy %s bugprone-branch-clone %t -- + +int test_asm1(int argc, char**) { + if (argc > 1) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone] + __asm__ volatile( + "addi %0, %0, -1" + : "+r" (argc) + ); + } else { +// CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here + __asm__ volatile( + "addi %0, %0, -1" + : "+r" (argc) + ); + } + return argc; +} + +int test_asm2(int argc, char**) { + if (argc > 1) { // no-warning + __asm__ volatile( + "addi %0, %0, -1" + : "+r" (argc) + ); + } else { + __asm__ volatile( + "addi %0, %0, -2" + : "+r" (argc) + ); + } + return argc; +} + +int test_asm3(int argc, char**) { + int Test1 = 0; + if (argc > 1) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone] + __asm__ volatile( + "add %w0, %w0, -1" + : "+r" (argc) + : "r" (Test1) + : "w0" + ); + } else { +// CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here + __asm__ volatile( + "add %w0, %w0, -1" + : "+r" (argc) + : "r" (Test1) + : "w0" + ); + } + return argc; +} + +int test_asm4(int argc, char**) { + int Test1 = 0; + if (argc > 1) { // no-warning + __asm__ volatile( + "add %w0, %w0, -1" + : "+r" (argc) + : "r" (Test1) + : "w0" + ); + } else { + __asm__ volatile( + "add %w0, %w0, -1" + : "+r" (argc) + : "r" (Test1) + : "w1" + ); + } + return argc; +} \ No newline at end of file >From eb11f9dedb5967b925f6edcfb116bad4c24cb998 Mon Sep 17 00:00:00 2001 From: LeeYoungJoon <[email protected]> Date: Wed, 10 Jun 2026 08:23:48 +0900 Subject: [PATCH 3/5] Fix test case to be target-independent --- .../bugprone/branch-clone-inline-asm.cpp | 48 ++++--------------- 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp index 05027fb18b5ab..1e12cbe43632d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp @@ -3,31 +3,19 @@ int test_asm1(int argc, char**) { if (argc > 1) { // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone] - __asm__ volatile( - "addi %0, %0, -1" - : "+r" (argc) - ); + __asm__ volatile("" : "+r" (argc)); } else { // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here - __asm__ volatile( - "addi %0, %0, -1" - : "+r" (argc) - ); + __asm__ volatile("" : "+r" (argc)); } return argc; } int test_asm2(int argc, char**) { if (argc > 1) { // no-warning - __asm__ volatile( - "addi %0, %0, -1" - : "+r" (argc) - ); + __asm__ volatile("foo" : "+r" (argc)); } else { - __asm__ volatile( - "addi %0, %0, -2" - : "+r" (argc) - ); + __asm__ volatile("bar" : "+r" (argc)); } return argc; } @@ -36,20 +24,10 @@ int test_asm3(int argc, char**) { int Test1 = 0; if (argc > 1) { // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if with identical then and else branches [bugprone-branch-clone] - __asm__ volatile( - "add %w0, %w0, -1" - : "+r" (argc) - : "r" (Test1) - : "w0" - ); + __asm__ volatile("" : "+r" (argc) : "r" (Test1) : "memory"); } else { // CHECK-MESSAGES: :[[@LINE-1]]:5: note: else branch starts here - __asm__ volatile( - "add %w0, %w0, -1" - : "+r" (argc) - : "r" (Test1) - : "w0" - ); + __asm__ volatile("" : "+r" (argc) : "r" (Test1) : "memory"); } return argc; } @@ -57,19 +35,9 @@ int test_asm3(int argc, char**) { int test_asm4(int argc, char**) { int Test1 = 0; if (argc > 1) { // no-warning - __asm__ volatile( - "add %w0, %w0, -1" - : "+r" (argc) - : "r" (Test1) - : "w0" - ); + __asm__ volatile("" : "+r" (argc) : "r" (Test1) : "memory"); } else { - __asm__ volatile( - "add %w0, %w0, -1" - : "+r" (argc) - : "r" (Test1) - : "w1" - ); + __asm__ volatile("" : "+r" (argc) : "r" (Test1) : "cc"); } return argc; } \ No newline at end of file >From 5d8c5a56656c2be91a785d386eb1aa73b22d9de9 Mon Sep 17 00:00:00 2001 From: LeeYoungJoon <[email protected]> Date: Wed, 10 Jun 2026 16:58:16 +0900 Subject: [PATCH 4/5] [clang-tidy] Address review comments for inline asm test --- .../clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp index 1e12cbe43632d..daf9b539ae50a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-inline-asm.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s bugprone-branch-clone %t -- +// RUN: %check_clang_tidy %s bugprone-branch-clone %t int test_asm1(int argc, char**) { if (argc > 1) { @@ -40,4 +40,4 @@ int test_asm4(int argc, char**) { __asm__ volatile("" : "+r" (argc) : "r" (Test1) : "cc"); } return argc; -} \ No newline at end of file +} >From 1d5ea1e77105f9ccacfe1b78decb5a7920b2d774 Mon Sep 17 00:00:00 2001 From: LeeYoungJoon <[email protected]> Date: Mon, 15 Jun 2026 14:51:37 +0900 Subject: [PATCH 5/5] [clang][modules] Add ODR test for inline-asm string and clobber profiling --- clang/test/Modules/asm-stmt-odr.cppm | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 clang/test/Modules/asm-stmt-odr.cppm diff --git a/clang/test/Modules/asm-stmt-odr.cppm b/clang/test/Modules/asm-stmt-odr.cppm new file mode 100644 index 0000000000000..e065f9211efdb --- /dev/null +++ b/clang/test/Modules/asm-stmt-odr.cppm @@ -0,0 +1,67 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -x c++ -std=c++20 %t/A.cppm -I%t -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -x c++ -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -x c++ -std=c++20 -fprebuilt-module-path=%t %t/use.cpp -verify +// +// RUN: rm %t/A.pcm %t/B.pcm +// RUN: %clang_cc1 -x c++ -std=c++20 %t/A.cppm -I%t -emit-reduced-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -x c++ -std=c++20 %t/B.cppm -I%t -emit-reduced-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -x c++ -std=c++20 -fprebuilt-module-path=%t %t/use.cpp -verify + +// Regression test for the StmtProfiler::VisitGCCAsmStmt fix in +// clang/lib/AST/StmtProfile.cpp (using Visit() so StringLiteral bytes are +// folded into the FoldingSetNodeID, and therefore into the ODR hash). +// Without the fix, two inline functions whose only difference is the inline +// asm body produce the same ODR hash, get merged across modules, and no +// diagnostic is emitted at the use site. + +//--- a.h +inline int asm_string_func() { + int x = 0; + __asm__("foo" : "+r"(x)); + return x; +} +inline int clobber_func() { + int x = 0; + __asm__("" : "+r"(x) : : "memory"); + return x; +} + +//--- a.v1.h +inline int asm_string_func() { + int x = 0; + __asm__("bar" : "+r"(x)); // differs from a.h: asm string + return x; +} +inline int clobber_func() { + int x = 0; + __asm__("" : "+r"(x) : : "cc"); // differs from a.h: clobber + return x; +} + +//--- A.cppm +module; +#include "a.h" +export module A; +export using ::asm_string_func; +export using ::clobber_func; + +//--- B.cppm +module; +#include "a.v1.h" +export module B; +export using ::asm_string_func; +export using ::clobber_func; + +//--- use.cpp +import A; +import B; +// expected-error@*:* 1+{{'asm_string_func' has different definitions in different modules}} +// expected-error@*:* 1+{{'clobber_func' has different definitions in different modules}} +// expected-note@*:* 1+{{but in 'A.<global>' found a different body}} + +int u1 = asm_string_func(); +int u2 = clobber_func(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
