[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] Fix crash when assiging to a property with an invalid type
From:       Bill Wendling <isanbard () gmail ! com>
Date:       2014-08-05 5:28:27
Message-ID: C0B3C5DA-A382-4D6B-AEBF-D14D19DC4FB7 () gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Okay. I added it in. Thanks!

-bw

On Aug 4, 2014, at 12:26 PM, jahanian <fjahanian@apple.com> wrote:

> 
> On Aug 4, 2014, at 12:11 PM, Olivier Goffart <ogoffart@kde.org> wrote:
> 
> > On Wednesday 30 July 2014 14:02:51 jahanian wrote:
> > > On Jul 30, 2014, at 1:43 PM, jahanian <fjahanian@apple.com> wrote:
> > > > On Jul 30, 2014, at 1:21 PM, Olivier Goffart <ogoffart@kde.org> wrote:
> > > > > It does crash for me (linux, r214316).
> > > > > In debug mode, there is an assert. In release mode it segfaults.
> > > > 
> > > > Strange, I tested it against a debuggable compiler (with assert on).
> > > > Let me investigate a bit more why it doesn’t crash for me.
> > > 
> > > OK. I missed that it was an ObjectiveC++ test. Both patches look good to me.
> > 
> > 
> > Thanks.
> > I committed them to trunk as r214734 and r214735.
> > I believe they should be merged in the 3.5 branch since they fix regression 
> > against 3.4, what do you think?
> 
> I am not sure what the bar is for fixing a crash on invalid code. If it is within \
>                 criteria then it is OK with me.
> - Fariborz
> 
> 
> > 
> > 
> > 
> > > > 
> > > > > > I don’t see a crash with TOT clang:
> > > > > > 
> > > > > > t.m:3:3: error: unknown type name 'A'
> > > > > > A* response; // expected-error {{unknown type name 'A'}}
> > > > > > ^
> > > > > > t.m:8:11: error: unknown type name 'A'
> > > > > > @property A* response;  // expected-error {{unknown type name 'A'}}
> > > > > > 
> > > > > > ^
> > > > > > 
> > > > > > t.m:13:16: error: expected a type
> > > > > > - (void) foo :(A*) a   // expected-error {{expected a type}}
> > > > > > 
> > > > > > ^
> > > > > > 
> > > > > > t.m:15:17: error: assignment to readonly property
> > > > > > self.response = a;
> > > > > > ~~~~~~~~~~~~~ ^ ~
> > > > > > 4 errors generated.
> > > > > > 
> > > > > > - Fariborz
> > > > > > 
> > > > > > On Jul 30, 2014, at 12:42 AM, Olivier Goffart <ogoffart@kde.org> wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > Attached you will find a fix for a crash I have when parsing objective
> > > > > > > C
> > > > > > > with missing include.
> > > > > > > This is a regression against clang 3.4 which did not crash.
> > > > > > > 
> > > > > > > My approach here is naive and just fixes the symptoms (the crash).
> > > > > > > 
> > > > > > > Regards
> > > > 
> > > > _______________________________________________
> > > > cfe-commits mailing list
> > > > cfe-commits@cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[Attachment #5 (unknown)]

<html><head><meta http-equiv="Content-Type" content="text/html \
charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; -webkit-line-break: after-white-space;">Okay. I added it in. \
Thanks!<div><br></div><div>-bw</div><div><br><div style=""><div>On Aug 4, 2014, at \
12:26 PM, jahanian &lt;<a \
href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>&gt; wrote:</div><br \
class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: \
12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: \
normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; \
text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; \
-webkit-text-stroke-width: 0px;"><br>On Aug 4, 2014, at 12:11 PM, Olivier Goffart \
&lt;<a href="mailto:ogoffart@kde.org">ogoffart@kde.org</a>&gt; \
wrote:<br><br><blockquote type="cite">On Wednesday 30 July 2014 14:02:51 jahanian \
wrote:<br><blockquote type="cite">On Jul 30, 2014, at 1:43 PM, jahanian &lt;<a \
href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>&gt; wrote:<br><blockquote \
type="cite">On Jul 30, 2014, at 1:21 PM, Olivier Goffart &lt;<a \
href="mailto:ogoffart@kde.org">ogoffart@kde.org</a>&gt; wrote:<br><blockquote \
type="cite">It does crash for me (linux, r214316).<br>In debug mode, there is an \
assert. In release mode it segfaults.<br></blockquote><br>Strange, I tested it \
against a debuggable compiler (with assert on).<br>Let me investigate a bit more why \
it doesn’t crash for me.<br></blockquote><br>OK. I missed that it was an ObjectiveC++ \
test. Both patches look good to me.<br></blockquote><br><br>Thanks.<br>I committed \
them to trunk as r214734 and r214735.<br>I believe they should be merged in the 3.5 \
branch since they fix regression<span \
class="Apple-converted-space">&nbsp;</span><br>against 3.4, what do you \
think?<br></blockquote><br>I am not sure what the bar is for fixing a crash on \
invalid code. If it is within criteria then it is OK with me.<br>- \
Fariborz<br><br><br><blockquote type="cite"><br><br><br><blockquote \
type="cite"><blockquote type="cite"><br><blockquote type="cite"><blockquote \
type="cite">I don’t see a crash with TOT clang:<br><br>t.m:3:3: error: unknown type \
name 'A'<br>A* response; // expected-error {{unknown type name \
'A'}}<br>^<br>t.m:8:11: error: unknown type name 'A'<br>@property A* response; \
&nbsp;// expected-error {{unknown type name \
'A'}}<br><br>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;^<br><br>t.m:13:16: error: expected \
a type<br>- (void) foo :(A*) a &nbsp;&nbsp;// expected-error {{expected a \
type}}<br><br>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;^<br><br>t.m:15:17: \
error: assignment to readonly property<br>self.response = a;<br>~~~~~~~~~~~~~ ^ \
~<br>4 errors generated.<br><br>- Fariborz<br><br>On Jul 30, 2014, at 12:42 AM, \
Olivier Goffart &lt;<a href="mailto:ogoffart@kde.org">ogoffart@kde.org</a>&gt; \
wrote:<br><blockquote type="cite">Hi,<br><br>Attached you will find a fix for a crash \
I have when parsing objective<br>C<br>with missing include.<br>This is a regression \
against clang 3.4 which did not crash.<br><br>My approach here is naive and just \
fixes the symptoms (the \
crash).<br><br>Regards<br></blockquote></blockquote></blockquote><br>_______________________________________________<br>cfe-commits \
mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><b \
r>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</blockquote></blockquote></blockquote></div></blockquote></div><br></div></body></html>




_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic