[libc++] Remove potential 0-sized array in __compressed_pair_padding by serge-sans-paille · Pull Request #109028 · llvm/llvm-project

serge-sans-paille · 2024-09-17T18:42:26Z

No description provided.

llvmbot · 2024-09-17T18:43:02Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-libcxx

Author: None (serge-sans-paille)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/109028.diff

1 Files Affected:

(modified) libcxx/include/__memory/compressed_pair.h (+6-4)

diff --git a/libcxx/include/__memory/compressed_pair.h b/libcxx/include/__memory/compressed_pair.h
index 629e3ad8848ffa..cc56912f5e34c5 100644
--- a/libcxx/include/__memory/compressed_pair.h
+++ b/libcxx/include/__memory/compressed_pair.h
@@ -52,13 +52,15 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 #ifndef _LIBCPP_ABI_NO_COMPRESSED_PAIR_PADDING
 
-template <class _ToPad>
+template <class _ToPad,
+          bool _Empty = ((is_empty<_ToPad>::value && !__libcpp_is_final<_ToPad>::value) || is_reference<_ToPad>::value)>
 class __compressed_pair_padding {
-  char __padding_[((is_empty<_ToPad>::value && !__libcpp_is_final<_ToPad>::value) || is_reference<_ToPad>::value)
-                      ? 0
-                      : sizeof(_ToPad) - __datasizeof_v<_ToPad>];
+  char __padding_[sizeof(_ToPad) - __datasizeof_v<_ToPad>];
 };
 
+template <class _ToPad>
+class __compressed_pair_padding<_ToPad, true> {};
+
 #  define _LIBCPP_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2)                                                  \
     _LIBCPP_NO_UNIQUE_ADDRESS __attribute__((__aligned__(_LIBCPP_ALIGNOF(T2)))) T1 Initializer1;                       \
     _LIBCPP_NO_UNIQUE_ADDRESS ::std::__compressed_pair_padding<T1> _LIBCPP_CONCAT3(__padding1_, __LINE__, _);          \

serge-sans-paille · 2024-09-17T18:44:03Z

In the spirit of #105865 - I don't think we can use a similar implementation as what we did there, but that's the same idea / motivation

ldionne · 2024-09-18T13:52:34Z

libcxx/include/__memory/compressed_pair.h

-  char __padding_[((is_empty<_ToPad>::value && !__libcpp_is_final<_ToPad>::value) || is_reference<_ToPad>::value)
-                      ? 0
-                      : sizeof(_ToPad) - __datasizeof_v<_ToPad>];
+  char __padding_[sizeof(_ToPad) - __datasizeof_v<_ToPad>];


Can't we reuse the same __padding struct we now have in <string>?

serge-sans-paille · 2024-09-20T08:10:52Z

It's not clear to me if the issue here come from the CI system or from my patch :-/

shafik

This needs a more detailed summary, e.g. what is the motivation for this change?

ldionne · 2024-09-25T17:30:25Z

Please rebase onto main to re-trigger CI. The CI instability should be resolved now.

github-actions · 2024-09-30T08:44:12Z

✅ With the latest revision this PR passed the C/C++ code formatter.

serge-sans-paille · 2024-09-30T09:27:02Z

@shafik : I'm trying to have libcxx compile with -Wzero-length-array. With these two commits, we're done.

@ldionne : I undid the reuse of __padding. The motivation is two folds: 1st it would create an extra header dependency (moving __padding definition to another header so that it can be included both in string and compressed_pair.h) and second compressed_pair is now scarcely used, so it's probably ok to keep related stuff in the same header (?).

…lvm#109028)

#109028 caused `sizeof` to be sometimes applied to function reference types, which makes a program ill-formed. This PR handles reference types by specializations to prevent such bogus `sizeof` expression to be instantiated. Fixes #142118.

…2516) This PR mirrors changes of `__compressed_pair_padding` in libc++ into lldb test suite. Related PR for libc++: - #108956 - #109028 - #142125

…m#142516) This PR mirrors changes of `__compressed_pair_padding` in libc++ into lldb test suite. Related PR for libc++: - llvm#108956 - llvm#109028 - llvm#142125 (cherry picked from commit 119705e)

…m#142516) This PR mirrors changes of `__compressed_pair_padding` in libc++ into lldb test suite. Related PR for libc++: - llvm#108956 - llvm#109028 - llvm#142125 (cherry picked from commit 119705e) (cherry picked from commit 205591b)

serge-sans-paille requested a review from a team as a code owner September 17, 2024 18:42

serge-sans-paille requested a review from ldionne September 17, 2024 18:42

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 17, 2024

serge-sans-paille force-pushed the feature/remove-empty-array branch 2 times, most recently from a64e67a to 0c70565 Compare September 18, 2024 09:32

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 18, 2024

serge-sans-paille force-pushed the feature/remove-empty-array branch from 0c70565 to dae40cd Compare September 18, 2024 10:29

ldionne requested changes Sep 18, 2024

View reviewed changes

shafik reviewed Sep 21, 2024

View reviewed changes

serge-sans-paille force-pushed the feature/remove-empty-array branch from 3d28f77 to 1112cfd Compare September 30, 2024 05:34

[libc++] Remove potential 0-sized array in __packed_format_arg_store

312cb4e

serge-sans-paille force-pushed the feature/remove-empty-array branch 2 times, most recently from ba11fce to cb61492 Compare September 30, 2024 08:40

serge-sans-paille force-pushed the feature/remove-empty-array branch 2 times, most recently from 87b87bd to 4ddd5a7 Compare September 30, 2024 09:02

[libc++] Remove potential 0-sized array in __compressed_pair_padding

0ab7824

serge-sans-paille force-pushed the feature/remove-empty-array branch from 4ddd5a7 to 0ab7824 Compare September 30, 2024 09:06

ldionne approved these changes Oct 1, 2024

View reviewed changes

ldionne merged commit 0eb2602 into llvm:main Oct 1, 2024

Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024

[libc++] Remove potential 0-sized array in __compressed_pair_padding (l…

14e6091

…lvm#109028)

This was referenced May 30, 2025

[libc++] Fix padding calculation for function reference types #142125

Merged

[lldb][test] Synchronize __compressed_pair_padding with libc++ #142516

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++] Remove potential 0-sized array in __compressed_pair_padding#109028

[libc++] Remove potential 0-sized array in __compressed_pair_padding#109028
ldionne merged 2 commits intollvm:mainfrom
serge-sans-paille:feature/remove-empty-array

serge-sans-paille commented Sep 17, 2024

Uh oh!

llvmbot commented Sep 17, 2024 •

edited

Loading

Uh oh!

serge-sans-paille commented Sep 17, 2024

Uh oh!

ldionne Sep 18, 2024

Uh oh!

serge-sans-paille Sep 18, 2024

Uh oh!

serge-sans-paille commented Sep 20, 2024

Uh oh!

shafik left a comment

Uh oh!

ldionne commented Sep 25, 2024

Uh oh!

github-actions bot commented Sep 30, 2024 •

edited

Loading

Uh oh!

serge-sans-paille commented Sep 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

serge-sans-paille commented Sep 17, 2024

Uh oh!

llvmbot commented Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serge-sans-paille commented Sep 17, 2024

Uh oh!

ldionne Sep 18, 2024

Choose a reason for hiding this comment

Uh oh!

serge-sans-paille Sep 18, 2024

Choose a reason for hiding this comment

Uh oh!

serge-sans-paille commented Sep 20, 2024

Uh oh!

shafik left a comment

Choose a reason for hiding this comment

Uh oh!

ldionne commented Sep 25, 2024

Uh oh!

github-actions bot commented Sep 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serge-sans-paille commented Sep 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

llvmbot commented Sep 17, 2024 •

edited

Loading

github-actions bot commented Sep 30, 2024 •

edited

Loading