[PATCH] D68115: Zero initialize padding in unions

tschuett added a comment. I watched the talk, but I still prefer compile-time errors over runtime crashes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68115/new/ https://reviews.llvm.org/D68115 ___ c

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. In D68115#2040696 , @tschuett wrote: > As an outsider: In Swift, reading an uninitialized variable is a compile-time > error. Clang is not powerful enough to do this analysis. Aren't you really > looking for the Clang Intermediate La

[PATCH] D68115: Zero initialize padding in unions

tschuett added a comment. As an outsider: In Swift, reading an uninitialized variable is a compile-time error. Clang is not powerful enough to do this analysis. Aren't you really looking for the Clang Intermediate Language (CIL) ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. To get this unblocked a bit, I implemented a diagnostic: https://reviews.llvm.org/D80055 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68115/new/ https://reviews.llvm.org/D68115 _

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1962892 , @rsmith wrote: > In D68115#1962863 , @jfb wrote: > > > In D68115#1962833 , @rsmith wrote: > > > > > I think the ma

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. > That sounds reasonable to me. So the behavior we're looking for is: > > - If `-ftrivial-auto-init` is off, then we guarantee to zero padding when the > language spec requires it, and otherwise provide no such guarantee. > - If `-ftrivial-auto-init=zeroes` then we guarantee

[PATCH] D68115: Zero initialize padding in unions

rsmith added a comment. In D68115#1962863 , @jfb wrote: > In D68115#1962833 , @rsmith wrote: > > > I think the majority opinion expressed on this review at this point favors > > not guaranteeing zero-initialization

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. In D68115#1962833 , @rsmith wrote: > In D68115#1946990 , @jfb wrote: > > > In D68115#1946757 , @rsmith wrote: > > > > > In D68115#1946668

[PATCH] D68115: Zero initialize padding in unions

rsmith added a comment. In D68115#1946990 , @jfb wrote: > In D68115#1946757 , @rsmith wrote: > > > In D68115#1946668 , > > @hubert.reinterpretcast wrote: > > > > > It sounds

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. In D68115#1946757 , @rsmith wrote: > In D68115#1946668 , > @hubert.reinterpretcast wrote: > > > It sounds like we are looking for `-fzero-union-padding`. That's been where > > the discussion h

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1946757 , @rsmith wrote: > 3. After doing (1), extend `__builtin_bit_cast` support to properly handle > padding bits. > 4. After doing (1) and (2), extend constant aggregate emission to always zero > padd

[PATCH] D68115: Zero initialize padding in unions

rsmith added a comment. In D68115#1946668 , @hubert.reinterpretcast wrote: > It sounds like we are looking for `-fzero-union-padding`. That's been where > the discussion has left off twice for months. I believe the state of Clang prior to this patch is

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1946612 , @jfb wrote: > What's the verdict then? It sounds like we are looking for `-fzero-union-padding`. That's been where the discussion has left off twice for months. Repository: rG LLVM Github M

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. What's the verdict then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68115/new/ https://reviews.llvm.org/D68115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D68115: Zero initialize padding in unions

aaron.ballman added a comment. In D68115#1820622 , @vitalybuka wrote: > In D68115#1820579 , @aaron.ballman > wrote: > > > In D68115#1820462 , @vitalybuka > > wrote: > > > >

[PATCH] D68115: Zero initialize padding in unions

vitalybuka added a comment. In D68115#1820579 , @aaron.ballman wrote: > In D68115#1820462 , @vitalybuka > wrote: > > > I would be happy to finish this patch if we agree on something. > > > > So if I understand thi

[PATCH] D68115: Zero initialize padding in unions

aaron.ballman added a comment. In D68115#1820462 , @vitalybuka wrote: > I would be happy to finish this patch if we agree on something. > > So if I understand this the proposal is to have something like > -fzero-union-padding which is off by default. > W

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1820462 , @vitalybuka wrote: > So if I understand this the proposal is to have something like > -fzero-union-padding which is off by default. > When it's OFF compiler will continue to do whatever it does

[PATCH] D68115: Zero initialize padding in unions

vitalybuka added a comment. In D68115#1819418 , @aaron.ballman wrote: > In D68115#1811091 , @lebedev.ri > wrote: > > > In D68115#1811089 , > > @hubert.reinterpretcast wrot

[PATCH] D68115: Zero initialize padding in unions

aaron.ballman added a comment. In D68115#1811091 , @lebedev.ri wrote: > In D68115#1811089 , > @hubert.reinterpretcast wrote: > > > In D68115#1810891 , @lebedev.ri > > wrote

[PATCH] D68115: Zero initialize padding in unions

lebedev.ri added a subscriber: aaron.ballman. lebedev.ri added a comment. In D68115#1811089 , @hubert.reinterpretcast wrote: > In D68115#1810891 , @lebedev.ri > wrote: > > > Does this have to be an unilateral chan

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1810891 , @lebedev.ri wrote: > Does this have to be an unilateral change, > likely penalizing non-`-ftrivial-auto-var-init=` cases, > i.e. [why] can't it be **only** done for when `-ftrivial-auto-var-init

[PATCH] D68115: Zero initialize padding in unions

lebedev.ri added a comment. Can patch description be made a bit more verbose? > However with -ftrivial-auto-var-init=pattern those undefs became 0xAA pattern > and break some code. Break how? Does this have to be an unilateral change, likely penalizing non-`-ftrivial-auto-var-init=` cases, i.e

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. @vitalybuka could we move this patch forward? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68115/new/ https://reviews.llvm.org/D68115 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1686887 , @vitalybuka wrote: > I would be happy to update the patch to enable it only for > -ftrivial-auto-var-init=pattern, if we want "bumper" version. It seems to be a separable feature (although it d

[PATCH] D68115: Zero initialize padding in unions

vitalybuka added a comment. > The patch as-is moves past the scope of the `-ftrivial-auto-var-init` feature. That's because I had misunderstanding of the standard. I would be happy to update the patch to enable it only for -ftrivial-auto-var-init=pattern, if we want "bumper" version. For "non-b

[PATCH] D68115: Zero initialize padding in unions

vitalybuka added a comment. In D68115#1686837 , @jfb wrote: > The entire point of this feature is to add guardrails to the language. What > do people expect in the real world? Is there a cost to meeting these > expectations? If we put the pattern (0x00 o

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1686837 , @jfb wrote: > The entire point of this feature is to add guardrails to the language. I agree, and guardrails have a tendency to scratch paint if one drives against them. > What do people expec

[PATCH] D68115: Zero initialize padding in unions

jfb added a comment. The entire point of this feature is to add guardrails to the language. What do people expect in the real world? Is there a cost to meeting these expectations? If we put the pattern (0x00 or 0xaa) in the technically undef space, what comes out? Repository: rG LLVM Github

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 false) + // CHECK: ca

[PATCH] D68115: Zero initialize padding in unions

vitalybuka marked 2 inline comments as done. vitalybuka added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 false) + // CHECK: ca

[PATCH] D68115: Zero initialize padding in unions

vitalybuka marked an inline comment as done. vitalybuka added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i

[PATCH] D68115: Zero initialize padding in unions

vitalybuka marked an inline comment as done. vitalybuka added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 false) + // CHECK: ca

[PATCH] D68115: Zero initialize padding in unions

vitalybuka updated this revision to Diff 222071. vitalybuka added a comment. remove unused var Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68115/new/ https://reviews.llvm.org/D68115 Files: clang/lib/CodeGen/CGExprConstant.cpp clang/test/Code

[PATCH] D68115: Zero initialize padding in unions

vitalybuka marked an inline comment as done. vitalybuka added inline comments. Comment at: clang/test/CodeGenCXX/designated-init.cpp:68 }; -// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } } +// CHECK-LE: @overwrite_padding = global { { i8, i8 }

[PATCH] D68115: Zero initialize padding in unions

vitalybuka marked an inline comment as done. vitalybuka added inline comments. Comment at: clang/test/CodeGenCXX/designated-init.cpp:68 }; -// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } } +// CHECK-LE: @overwrite_padding = global { { i8, i8 }

[PATCH] D68115: Zero initialize padding in unions

vitalybuka created this revision. vitalybuka added reviewers: rsmith, jfb. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. vitalybuka edited the summary of this revision. vitalybuka edited the summary of this revision. Existing implementation puts undef into paddi