DKLoehr wrote:
Thanks @zmodem for merging (and also reverting). Follow-up PR with the fix is
#125526.
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
zmodem wrote:
Looks like some buildbots are unhappy. I'll back it out for now.
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvm-ci wrote:
LLVM Buildbot has detected a new failure on builder `llvm-clang-x86_64-sie-win`
running on `sie-win-worker` while building `clang` at step 7
"test-build-unified-tree-check-all".
Full details are available at:
https://lab.llvm.org/buildbot/#/builders/46/builds/11501
Here is th
llvm-ci wrote:
LLVM Buildbot has detected a new failure on builder
`llvm-clang-x86_64-sie-ubuntu-fast` running on `sie-linux-worker` while
building `clang` at step 6 "test-build-unified-tree-check-all".
Full details are available at:
https://lab.llvm.org/buildbot/#/builders/144/builds/17222
github-actions[bot] wrote:
@DKLoehr Congratulations on having your first Pull Request (PR) merged into the
LLVM Project!
Your changes will be combined with recent changes from other authors, then
tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a
problem with a build,
https://github.com/zmodem closed
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/zmodem approved this pull request.
lgtm
Unless there are any further comments, I'll push the Merge button on Monday.
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://li
@@ -13374,6 +13374,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *Dcl) {
+ if (!Dcl || !getLangOpts().CPlusPlus)
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 798b3f21593499194487c9877b8f4a3d9e44bb6e Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/5] Warn when unique objects might be duplicated in shared
librar
@@ -6153,6 +6153,15 @@ def warn_static_local_in_extern_inline : Warning<
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
+def warn_possible_object_duplication_mutable : Warning<
+ "%0 is mutable, has hidden visibility,
zmodem wrote:
Sounds good. This basically seems fine to me overall. I've just added two more
comments.
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/list
@@ -6153,6 +6153,15 @@ def warn_static_local_in_extern_inline : Warning<
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
+def warn_possible_object_duplication_mutable : Warning<
+ "%0 is mutable, has hidden visibility,
@@ -13374,6 +13374,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *Dcl) {
+ if (!Dcl || !getLangOpts().CPlusPlus)
DKLoehr wrote:
Addressed feedback, responded where appropriate. I've also done some digging
into how the warning should work on Windows: it seems like it'll be very
straightforward (just replace "has hidden visibility" with "doesn't have a dll
import/export attribute"), but I'll leave that for
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 798b3f21593499194487c9877b8f4a3d9e44bb6e Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/4] Warn when unique objects might be duplicated in shared
librar
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 798b3f21593499194487c9877b8f4a3d9e44bb6e Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/5] Warn when unique objects might be duplicated in shared
librar
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 798b3f21593499194487c9877b8f4a3d9e44bb6e Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/4] Warn when unique objects might be duplicated in shared
librar
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
https://github.com/DKLoehr edited
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -6153,6 +6153,15 @@ def warn_static_local_in_extern_inline : Warning<
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
+def warn_possible_object_duplication_mutable : Warning<
+ "%0 is mutable, has hidden visibility,
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -13374,6 +13374,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *Dcl) {
+ if (!Dcl || !getLangOpts().CPlusPlus)
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -3661,6 +3661,12 @@ class Sema final : public SemaBase {
NonTrivialCUnionContext UseContext,
unsigned NonTrivialKind);
+ /// Certain globally-unique variables might be accidentally duplicated if
+ /// built into mu
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -0,0 +1,187 @@
+/**
+ * When building shared libraries, hidden objects which are defined in header
+ * files will be duplicated, with one copy in each shared library. If the
object
+ * was meant to be globally unique (one copy per program), this can cause very
+ * subtle bugs.
@@ -6153,6 +6153,15 @@ def warn_static_local_in_extern_inline : Warning<
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
+def warn_possible_object_duplication_mutable : Warning<
+ "%0 is mutable, has hidden visibility,
@@ -508,6 +508,10 @@ New Compiler Flags
- clang-cl and clang-dxc now support
``-fdiagnostics-color=[auto|never|always]``
in addition to ``-f[no-]color-diagnostics``.
+- The ``-Wunique-object-duplication`` warning has been added to warn about
objects
zmodem
@@ -6153,6 +6153,15 @@ def warn_static_local_in_extern_inline : Warning<
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
+def warn_possible_object_duplication_mutable : Warning<
+ "%0 is mutable, has hidden visibility,
@@ -13374,6 +13374,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *Dcl) {
+ if (!Dcl || !getLangOpts().CPlusPlus)
DKLoehr wrote:
> Can you share a few of the examples where it triggers, to get a feel for what
> this looks like on the LLVM code?
There's a text file linked in the initial description
([here](https://github.com/user-attachments/files/18563888/clang-warnings.txt),
for convenience) that summar
rnk wrote:
+1 to separating implementation from enablement. Enabling the warning by
default will predictably generate pushback and folks may ask to revert, and
it's a good best practice to be able to easily do that while leaving the
implementation in place.
https://github.com/llvm/llvm-projec
zmodem wrote:
> I intend to make the warning on-by-default. However, it currently triggers at
> several places in the LLVM project, breaking the CI. Fixing them is
> nontrivial and cluttered up the PR, so I made it DefaultIgnore in order to
> keep things focused. Should the PR be accepted I pl
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 5ef8b8f3b84133ac7501331bf9b86b0b2f8b9ed9 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/2] Warn when unique objects might be duplicated in shared
librar
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 5ef8b8f3b84133ac7501331bf9b86b0b2f8b9ed9 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/3] Warn when unique objects might be duplicated in shared
librar
DKLoehr wrote:
I've now added documentation for this warning explaining what it means and how
to resolve it. I also added a release note mentioning its existence.
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commit
DKLoehr wrote:
@zmodem Thanks for the review! I've addressed the code comments. To your
questions:
> What about code that's not going into a shared library?
My impression is that you wouldn't mark things as having hidden visibility if
they're not going to a shared library at least some of the
DKLoehr wrote:
If a review was requested from you and this code isn't related, apologies: For
some reason attempting to rebase onto the current state of the repo ended up
including a bunch of random extra commits that aren't related, and that
resulted in a bunch of people automatically being a
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 5ef8b8f3b84133ac7501331bf9b86b0b2f8b9ed9 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/2] Warn when unique objects might be duplicated in shared
librar
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From 5ef8b8f3b84133ac7501331bf9b86b0b2f8b9ed9 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH 1/6] Warn when unique objects might be duplicated in shared
librar
@@ -13386,6 +13386,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *dcl) {
+ if (!dcl || !getLangOpts().CPlusPlus)
@@ -13386,6 +13386,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *dcl) {
+ if (!dcl || !getLangOpts().CPlusPlus)
+return fals
@@ -13386,6 +13386,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *dcl) {
+ if (!dcl || !getLangOpts().CPlusPlus)
+return fals
https://github.com/zmodem edited
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/zmodem commented:
Some scattered thoughts:
- What about code that's not going into a shared library. Should we disable the
warning then, or is the idea that the user does not enable the warning flag in
that case?
- The new warning is DefaultIgnore. Do you figure it's too no
@@ -13386,6 +13386,62 @@ void Sema::checkNonTrivialCUnion(QualType QT,
SourceLocation Loc,
.visit(QT, nullptr, false);
}
+bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
+const VarDecl *dcl) {
+ if (!dcl || !getLangOpts().CPlusPlus)
rnk wrote:
I added some reviewers to increase visibility. This warning is designed to
avoid common pitfalls with shared objects and hidden visibility as I understand
it, so I thought @maskray might be able to help with some ELF linker/loader
insight as to whether this warning is valuable.
htt
zmodem wrote:
I'll try to take a look. Also, @ilya-biryukov do you have someone who might
want to take a look?
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mail
DKLoehr wrote:
Ping
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
DKLoehr wrote:
Ping
https://github.com/llvm/llvm-project/pull/117622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
DKLoehr wrote:
Because this warning keeps firing in many different places, including in
third-party libraries, I've disabled it by default until I can go through and
fix all the instances. I think this provides a cleaner separation of concerns,
so that this PR isn't clogged up with various hou
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From ba531f3dce1b992dad191e5a9f724ebdc6750d6c Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH] Warn when unique objects might be duplicated in shared
libraries
https://github.com/DKLoehr updated
https://github.com/llvm/llvm-project/pull/117622
>From d944b2fde573a4fb352400ce3425121265b02685 Mon Sep 17 00:00:00 2001
From: Devon Loehr
Date: Thu, 21 Nov 2024 19:29:00 +
Subject: [PATCH] Warn when unique objects might be duplicated in shared
libraries
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Devon Loehr (DKLoehr)
Changes
When a hidden object is built into multiple shared libraries, each instance of
the library will get its own copy. If
the object was supposed to be globally unique (e.g. a global variable or static
data member
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you, it
https://github.com/DKLoehr created
https://github.com/llvm/llvm-project/pull/117622
When a hidden object is built into multiple shared libraries, each instance of
the library will get its own copy. If
the object was supposed to be globally unique (e.g. a global variable or static
data member),
59 matches
Mail list logo
