[cfe-commits] r148243 - /cfe/trunk/lib/CodeGen/CGStmt.cpp

[Eli Friedman]

On Mon, Jan 16, 2012 at 10:14 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Mon, Jan 16, 2012 at 11:56 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Mon, Jan 16, 2012 at 9:49 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>> On Mon, Jan 16, 2012 at 9:35 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>>>> Author: fjahanian
>>>> Date: Mon Jan 16 11:35:57 2012
>>>> New Revision: 148243
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=148243&view=rev
>>>> Log:
>>>> Fixes a crash during constant folding of a switch and case
>>>> statement which has an unscoped case inside it.
>>>> Patch by Aaron Ballman
>>>>
>>>> Modified:
>>>>    cfe/trunk/lib/CodeGen/CGStmt.cpp
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=148243&r1=148242&r2=148243&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Mon Jan 16 11:35:57 2012
>>>> @@ -878,6 +878,16 @@
>>>>  }
>>>>
>>>>  void CodeGenFunction::EmitCaseStmt(const CaseStmt &S) {
>>>> +  // If there is no enclosing switch instance that we're aware of, then this
>>>> +  // case statement and its block can be elided.  This situation only happens
>>>> +  // when we've constant-folded the switch, are emitting the constant case,
>>>> +  // and part of the constant case includes another case statement.  For
>>>> +  // instance: switch (4) { case 4: do { case 5: } while (1); }
>>>> +  if (!SwitchInsn) {
>>>> +    EmitStmt(S.getSubStmt());
>>>> +    return;
>>>> +  }
>>>
>>> I'm not convinced that this patch does the right thing if the given
>>> construct is nested into another switch.
>>
>> Ah - good point. I'd only thought about having the constant folding
>> stuff remove the case labels for cleanliness - but it might be a
>> matter of correctness too, as you've pointed out/hinted at. (yay for
>> the 'nice' thing being the right thing too)
>>
>> I was/am meaning to look at this alternative approach, but since it
>> may be a matter of correctness, perhaps Aaron will be motivated to
>> test/investigate/fix it if I don't get around to it.
>
> I'll come up with some test cases to check this out tonight, and pass
> them along to verify I understand the concerns properly.

Okay... I'm specifically concerned about the code generation for
non-constant-folded-switch containing a constant-folded switch.

-Eli