[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D81678#2099444 , @efriedma wrote:

> So I guess we've discussed the following alternatives so far:
>
> (snip)
>
> Maybe (1) is the least-bad; all the others compromise by making LLVM harder 
> to understand.  We can make porting the clang tests easier by adding a cc1 
> flag to turn off emitting frozen attributes, I guess (so instead of updating 
> the CHECK lines, you could just mechanically update the RUN line).




In D81678#2099591 , @eugenis wrote:

> I agree that (1) is the easiest to work with and the least error-prone, and 
> that's what we must shoot for in the design. We could do (4) later as an 
> optimization.


I agree. After going to (1) we can bring optimizations as a follow-up if it 
needed. I vote for the -cc1 option as a workaround.
BTW, was there a case in the past that needed a massive updates in tests as 
well? I wonder how it was addressed at that time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

In an email conversation with @rsmith and @eugenis, they raised the issue that 
it's not necessarily wrong to pass aggregate types by value, even when some 
fields are uninit.

A relevant excerpt from Richard:

> In addition to the union case, there's another strange case for passing class 
> types: in C++17 onwards, this example:
> 
> void f(S s);
>  void g() { f(S()); }
> 
> ... results in the parameter to f being constructed in place, rather than by 
> calling the copy constructor. (The same happens for a call to "f({})" in 
> C++11 onwards; it's just a lot more common in C++17.) So it is not the case 
> in general that f cannot be called with a partially-uninitialized S object. 
> :-(

From this, it's probably best for now not to mark aggregate types `frozen`. 
There's potentially some means to have the compiler prove aggregates must be 
frozen, but it's best to land frozen scalars first. And for the purposes of 
msan at least, scalars can account for the majority of the optimizations coming 
out of frozen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

In D81678#2099444 , @efriedma wrote:

> So I guess we've discussed the following alternatives so far:
>
> 1. Attach the "frozen" attribute everywhere; this makes the textual IR 
> generated by clang messy, and likely bloats memory usage (not sure by how 
> much).
> 2. Invert the meaning of the attribute; this makes reasoning about it messy.
> 3. Have a "frozen" attribute, but have a function attribute "frozen_args" to 
> freeze all arguments.  This is slightly messy to access from C++, and messy 
> to modify from C++.
> 4. Choose one of the previous three for memory, and a different one for 
> textual IR, and do some magic to translate.  This makes it harder to 
> understand the in-memory representation from reading textual IR.
>
>   I'm not particularly happy with any of these...
>
>   Maybe (1) is the least-bad; all the others compromise by making LLVM harder 
> to understand.  We can make porting the clang tests easier by adding a cc1 
> flag to turn off emitting frozen attributes, I guess (so instead of updating 
> the CHECK lines, you could just mechanically update the RUN line).


I agree that (1) is the easiest to work with and the least error-prone, and 
that's what we must shoot for in the design. We could do (4) later as an 
optimization.

Not sure about the cc1 flag - it's an option, but it would mean we are not 
testing the same thing that is shipped to the users. It will make things a lot 
easier, on the other hand.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

So I guess we've discussed the following alternatives so far:

1. Attach the "frozen" attribute everywhere; this makes the textual IR 
generated by clang messy, and likely bloats memory usage (not sure by how much).
2. Invert the meaning of the attribute; this makes reasoning about it messy.
3. Have a "frozen" attribute, but have a function attribute "frozen_args" to 
freeze all arguments.  This is slightly messy to access from C++, and messy to 
modify from C++.
4. Choose one of the previous three for memory, and a different one for textual 
IR, and do some magic to translate.  This makes it harder to understand the 
in-memory representation from reading textual IR.

I'm not particularly happy with any of these...

Maybe (1) is the least-bad; all the others compromise by making LLVM harder to 
understand.  We can make porting the clang tests easier by adding a cc1 flag to 
turn off emitting frozen attributes, I guess (so instead of updating the CHECK 
lines, you could just mechanically update the RUN line).

Maybe worth sending an email to llvm-dev.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

Adding the function attribute turns out to have other challenges. Specifically, 
we don't want to have transforms remove a `frozen` from a parameter and then 
have to go through and update all the other parameters. This might happen if 
the function is marked with `frozen_args`; something like `ArgumentPromotion`, 
which usually removes attributes, would have to be aware of it.

I spoke with Evgenii about having a "magic" function attribute that's only 
really exposed at the bitcode/text level. It would get converted to/from the 
individual parameter `frozen`s so that transforms don't need to know or care 
that it exists. `frozen_args` would only serve as a sort of compression keyword 
in the bitcode/text IR. He wasn't thrilled with the idea but suggested I get 
peoples' thoughts on it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In terms of the C++ API, we definitely want to provide an API phrased 
positively in terms of individual arguments, so transforms don't have to deal 
with inverted logic.

In terms of the actual internal memory representation, or textual IR, maybe we 
can be a bit more flexible.  `args_frozen` might be a reasonable compromise; 
the memory overhead should be pretty minimal even if we're attaching it to a 
bunch of functions, and it should be mostly readable.  But maybe a little 
awkward to explain the textual IR if it has both `arg_frozen` and `args_frozen`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

In D81678#2097081 , @aqjune wrote:

> To minimize diff, what about additionally introducing a function-level 
> attribute such as `args_frozen` stating that all arguments are frozen. (e.g 
> `define void @f(i32 x, i32 y) args_frozen`)?


I like this idea, and I think it will vastly reduce the tests diff. I'll try to 
update this patch with a positive `frozen` arg attribute and `args_frozen` 
function attribute ASAP.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D81678#2091089 , @eugenis wrote:

> Positive attribute sounds good to me (frozen is not a bad name), but the 
> tests update change is going to be huge. Any concerns about IR size bloat? 
> The attribute will apply to the majority of function arguments, 8 bytes per 
> instance as far as I can see.


Hi,

The issue stems from the difference between whether passing poison/undef to fn 
arg is allowed in C and IR, so (to add a positive attribute) increase in diff 
and # of attributes will happen, the question is how we can make the increase 
as small as possible.
To minimize diff, what about additionally introducing a function-level 
attribute such as `args_frozen` stating that all arguments are frozen. (e.g 
`define void @f(i32 x, i32 y) args_frozen`)?
The attribute will appear at the end of the line, so many CHECK of function 
signatures will still pass. (e.g. clang/test/CodeGen/mips64-padding-arg.c )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Positive attribute sounds good to me (frozen is not a bad name), but the tests 
update change is going to be huge. Any concerns about IR size bloat? The 
attribute will apply to the majority of function arguments, 8 bytes per 
instance as far as I can see.

Good point about uninitialized variables being undef, not poison. The meaning 
of the attribute should be "poison or undef (even partial) in this argument is 
UB". An extension that specifies which bytes, or even bits, of the argument are 
required to be undef-free would be nice, but seems too much at this point.

Another way to design this thing that would work for MSan, but not so much for 
general poison/undef analysis, is to emit an intrinsic call on the pre-coerced 
type to force an MSan check, and then an attribute ("partialinit") on the 
argument to skip checking the coerced value. This way the frontend has greater 
control over which parts of the arguments correspond to C++-things, and what 
does it mean for them to be initialized. I'm not sure I like this design though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D81678#2089041 , @aqjune wrote:

> > @efriedma 
> >  The way that call argument coercion works is unsound in the presence of 
> > poison. An integer can't be partially poisoned: it's either poison, or not 
> > poison. We probably need to come up with some safer way to pass 
> > structs/unions.
>
> This is true, clang frontend may lower an argument with aggregate type into 
> one with large int type (such as i64).
>  However, can poison value be safely generated in C? Paddings or union with 
> different size may contain undef bits, but not poison. Signed overflow is UB.
>  Undef value can exist bitwisely, so I think this is an orthogonal issue.


In C semantics, an expression can't produce a poison value.  As long as 
variables and allocations are initialized to undef, not poison, there isn't any 
way to sneak poison into the padding of a variable, so argument passing is 
sound.  So I guess it's not an issue unless we start poisoning uninitialized 
variables.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added inline comments.



Comment at: clang/include/clang/AST/Type.h:2139-2141
+  /// Check if this type has only two possible values, and so may be lowered to
+  /// a bool.
+  bool hasBooleanRepresentation() const;

rsmith wrote:
> This seems like a CodeGen-specific concern; I'm not sure this makes sense as 
> a query on the Type.
Makes sense, I can move it.



Comment at: clang/lib/AST/Type.cpp:2752-2753
+
+  if (const EnumType *ET = getAs())
+return ET->getDecl()->getIntegerType()->isBooleanType();
+

rsmith wrote:
> Under `-fstrict-enums` in C++, `enum E { a = 0, b = 1 };` has only two 
> distinct valid values. Should we consider that case?
I factored this code out of CGExpr.cpp, as it looked like this function 
governed whether `i1` values were lifted to `i8` (to meet the requirements of 
`bool`). I wanted to avoid struct `bool` members being marked `partialinit`. Do 
you think that would be a worthwhile separate change?



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:679
 
+void CGRecordLowering::determineMemberPartialInit() {
+  auto hasTailPadding = [&](QualType FieldTypePtr) {

rsmith wrote:
> We already have support for C++'s `__has_unique_object_representations` 
> (`ASTContext::hasUniqueObjectRepresentations`), which does something very 
> similar to this. Do we need both? (Are there cases where this is 
> intentionally diverging from `__has_unique_object_representations`?)
For the purposes of this commit, I think I can change the decisions made in 
CGRecordLayoutBuilder to defer to `ASTContext::hasUniqueObjectRepresentations`. 
In an upcoming change though, I'd like to determine exactly what *kind* of 
padding is present in the field, most importantly between basic field padding 
and bitfield/union padding. The reasoning is as follows:

- Union padding (e.g. `union { i32 a; i8 b; }`) is completely erased from the 
outgoing LLVM type, as it's lowered to a `{ i32 }`; same thing goes for 
bitfield tail padding.
- On the other hand, field padding (e.g. in `struct { i8 a; i16 b; }`) is 
preserved in the lowered LLVM type `{ i8, i16 }`.

This means that under certain CGCall conventions, if we only observe field 
padding, we can get away with not using `partialinit`.
- If we're flattening a struct (without coercion) that contains no "lost" 
padding, then each function argument will be a fully initialized field of the 
struct.
- Same thing goes if we're returning an uncoerced LLVM type such as `{ i8, i16 
}`: each individual field is still present and fully initialized.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

Hi, this is really interesting. I was interested in statically analyzing 
whether a value is undef/poison, so I also thought about the existence of this 
attribute, but I never imagined that MSan would benefit from this attribute as 
well.

> The partialinit attribute is, in some sense, backwards: the definition is 
> essentially "an argument *not* marked partialinit must not contain any poison 
> values". We usually try to avoid negative reasoning like this; I'm afraid 
> it'll make transforms harder to reason about.



> An alternative is to invert the meaning of the attribute and put it on all 
> arguments that must be not poison. Those are a lot more common though.

I agree with these two opinions. IIUC, in LLVM IR, attaching attribute to an 
argument imposes more restriction to the value.
Changing the semantics of IR to raise UB when poison or undef is passed to a 
function call will affect soundness of optimizations as well: for example, 
function outlining or dead arg elimination can introduce such function calls.
If the change is big, maybe we can split the patch to (1) implement the 
attribute (2) enable adding the attribute, so (2) contains all the big & 
mechanical diffs.

> @efriedma 
>  The way that call argument coercion works is unsound in the presence of 
> poison. An integer can't be partially poisoned: it's either poison, or not 
> poison. We probably need to come up with some safer way to pass 
> structs/unions.

This is true, clang frontend may lower an argument with aggregate type into one 
with large int type (such as i64).
However, can poison value be safely generated in C? Paddings or union with 
different size may contain undef bits, but not poison. Signed overflow is UB.
Undef value can exist bitwisely, so I think this is an orthogonal issue.

> @rsmith 
> It's not entirely clear to me what partialinit means. Does it mean that some 
> bytes of the parameter might be undef / poison? Or does it mean that some 
> bytes of the parameter that (for a struct parameter or array thereof) 
> correspond to a struct member might be undef / poison? (Eg, if I pass a { i8, 
> i32 } to a function, do we need to explicitly say that the three padding 
> bytes are not initialized?)

For poison, I believe it is now used as a valuewise concept in LLVM, so if the 
argument is a non-aggregate type such as i32/float/i8* it should be just binary 
(whether it is poison or not).

If it is agreed that we should have inverted version of `partialinit`, I'd like 
to suggest calling it `frozen` instead... :)
We have the notion of frozen value in LangRef already: it is used as e.g. `the 
branch condition should be frozen, otherwise it is undefined behavior`.
If an argument is frozen, it does not have any undef bits or poison value.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

As it stands, this attribute is applied whether or not msan is enabled, 
specifically because we think it can be useful in other contexts.

As for the negativity of this attribute, it's true that it would be more 
intuitive to have it be something like `fullinit` instead. I did it this way 
because passing arguments which are `partialinit` is orders of magnitude less 
common than the inverse. So this avoids polluting most generated code with a 
new attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

It's not entirely clear to me what `partialinit` means. Does it mean that some 
bytes of the parameter might be undef / poison? Or does it mean that some bytes 
of the parameter that (for a struct parameter or array thereof) correspond to a 
struct member might be undef / poison? (Eg, if I pass a `{ i8, i32 }` to a 
function, do we need to explicitly say that the three padding bytes are not 
initialized?)

I agree with @efriedma that a positive property, something like "these byte / 
bit ranges within the parameter are not undef", would probably fit better, and 
are likely usable by optimization passes as well as by msan. (Can we put 
metadata on parameters yet? This would seem well-suited to being modeled as 
metadata.)




Comment at: clang/include/clang/AST/Type.h:2139-2141
+  /// Check if this type has only two possible values, and so may be lowered to
+  /// a bool.
+  bool hasBooleanRepresentation() const;

This seems like a CodeGen-specific concern; I'm not sure this makes sense as a 
query on the Type.



Comment at: clang/lib/AST/Type.cpp:2752-2753
+
+  if (const EnumType *ET = getAs())
+return ET->getDecl()->getIntegerType()->isBooleanType();
+

Under `-fstrict-enums` in C++, `enum E { a = 0, b = 1 };` has only two distinct 
valid values. Should we consider that case?



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:679
 
+void CGRecordLowering::determineMemberPartialInit() {
+  auto hasTailPadding = [&](QualType FieldTypePtr) {

We already have support for C++'s `__has_unique_object_representations` 
(`ASTContext::hasUniqueObjectRepresentations`), which does something very 
similar to this. Do we need both? (Are there cases where this is intentionally 
diverging from `__has_unique_object_representations`?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

In D81678#2088284 , @efriedma wrote:

> I usually like to start reading this sort of patch with the proposed LangRef 
> change, but I'm not seeing one.
>
> There are a couple of related issues here in the existing representation of 
> IR:
>
> 1. The way that call argument coercion works is unsound in the presence of 
> poison.  An integer can't be partially poisoned: it's either poison, or not 
> poison.  We probably need to come up with some safer way to pass 
> structs/unions.
> 2. We don't currently have a way for frontends to indicate that a value is 
> guaranteed not to be poison, so we have to conservatively assume arguments 
> might be poison.  Whatever solution we come up with here should apply whether 
> or not msan is enabled.  An attribute like this will probably be useful for 
> "freeze" optimizations.
>
>   The partialinit attribute is, in some sense, backwards: the definition is 
> essentially "an argument *not* marked partialinit must not contain any poison 
> values".  We usually try to avoid negative reasoning like this; I'm afraid 
> it'll make transforms harder to reason about.


Yes, this is a bit awkward. There is a module flag 
"DisallowPoisonedCallArguments" that basically confirms that the frontend is 
aware of partialinit and any arguments not marked as such can be assumed strict 
(no-poison).

An alternative is to invert the meaning of the attribute and put it on all 
arguments that must be not poison. Those are a lot more common though.

The idea is that MSan can apply strict checking to arguments that are not 
"partialinit", and propagate the shadow to the callee for the rest.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a reviewer: aqjune.
efriedma added a comment.
Herald added a subscriber: wuzish.

I usually like to start reading this sort of patch with the proposed LangRef 
change, but I'm not seeing one.

There are a couple of related issues here in the existing representation of IR:

1. The way that call argument coercion works is unsound in the presence of 
poison.  An integer can't be partially poisoned: it's either poison, or not 
poison.  We probably need to come up with some safer way to pass structs/unions.
2. We don't currently have a way for frontends to indicate that a value is 
guaranteed not to be poison, so we have to conservatively assume arguments 
might be poison.  Whatever solution we come up with here should apply whether 
or not msan is enabled.  An attribute like this will probably be useful for 
"freeze" optimizations.

The partialinit attribute is, in some sense, backwards: the definition is 
essentially "an argument *not* marked partialinit must not contain any poison 
values".  We usually try to avoid negative reasoning like this; I'm afraid 
it'll make transforms harder to reason about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81678/new/

https://reviews.llvm.org/D81678



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits