[PATCH] Make sure BitcodeWriter works with Unicode characters

Keno Fischer kfischer at college.harvard.edu
Fri Dec 12 15:57:12 PST 2014


Ok, thanks for taking a look. I'll have a look again and see if I can
expose something.

On Fri, Dec 12, 2014 at 6:56 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> Oversight.  TBH, I didn't look at your patch again; I just saw the PR,
> remembered you'd fixed the problem before, and looked up the thread in
> order to credit you (and to find the right paint colour).
>
> Just now, I tried to expose a few other problems with testcases, and they
> all seem to be invalid (`llvm-as` doesn't accept strange characters in
> metadata names) or already work (names of globals, structs, and locals).
>
> Go ahead and commit (or file a PR) for anything you can make a testcase
> for though!
>
> > On 2014 Dec 12, at 15:08, Keno Fischer <kfischer at college.harvard.edu>
> wrote:
> >
> > Hi Duncan,
> > looking at your commit, were the other three instances that were in my
> patch of this pattern not affected, or was that an oversight?
> >
> > Thanks,
> > Keno
> >
> > On Thu, Dec 11, 2014 at 6:53 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > No problem!
> >
> > > On 2014 Dec 11, at 15:52, Keno Fischer <kfischer at college.harvard.edu>
> wrote:
> > >
> > > Sorry about that. I haven't found the time to get back to this (super
> busy this last month). I was planning to look at this again next week, but
> thanks for committing it :)!
> > >
> > > On Thu, Dec 11, 2014 at 6:38 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > >
> > > > On 2014 Nov 9, at 13:17, Keno Fischer <kfischer at college.harvard.edu>
> wrote:
> > > >
> > > > Previously when a metadata string contained unicode characters,
> > > > it would be incorrectly placed in the Record array because chars
> > > > are signed by default and hence characters with the high bit set
> > > > would get sign extended, but the bitcode writer was attempting
> > > > to write the lowest 8 bit of the now sign-extended value. This
> > > > caused an assertion failure later on. The fix is just to cast
> > > > the pointer to uint8_t* first to prevent sign extension.
> > > > This came up in the context for metadata strings, but I did a
> > > > quick pass and changed the other instances of this pattern in
> > > > the file as well.
> > > >
> > > > http://reviews.llvm.org/D6184
> > > >
> > > > Files:
> > > >  lib/Bitcode/Writer/BitcodeWriter.cpp
> > > >  test/Bitcode/unicode.ll
> > > > <D6184.15959.patch>_______________________________________________
> > > > llvm-commits mailing list
> > > > llvm-commits at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > > This review seemed to get stalled.  I just committed a fix based on
> Keno's
> > > patch in r224077 (which I think matches what David and my bike-shedding
> > > landed on), fixing PR21882.
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141212/94297c50/attachment.html>


More information about the llvm-commits mailing list