[PATCH] Make sure BitcodeWriter works with Unicode characters

Duncan P. N. Exon Smith dexonsmith at apple.com
Fri Dec 12 15:56:16 PST 2014


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.
> >
> 
> 





More information about the llvm-commits mailing list