[cfe-commits] PATCH: wide strings alignment fix in clang

[Sundeep]

Hi Craig,

I liked your idea. I verified that it fixes my test case. I also improved
your original patch as follows:

1. Updated 'GetAddrOfConstantString' and 'GetAddrOfConstantCString' APIs to
default assign Alignment to 1.
2. Undid your changes to all call sites calling those APIs with alignment of
1.

The new patch is attached. John, please let me know if the new patch looks
ok.

Thanks,
Sundeep

> -----Original Message-----
> From: Craig Topper [mailto:craig.topper at gmail.com]
> Sent: Wednesday, August 03, 2011 2:15 AM
> To: cfe-commits at cs.uiuc.edu; sundeepk at codeaurora.org
> Subject: [cfe-commits] PATCH: wide strings alignment fix in clang
> 
> The previous patch no longer applies due to the my unicode string
> literal patch that went in last week. I took a stab at a new patch.
> Rather than using the different string kinds to get the proper
> alignment, I queried ASTContext for the alignment of the type contained
> in the StringLiteral. I push the alignment as an argument down to the
> functions that actually look up the string in the map. If we find a
> match in the map, I max the old alignment and the new alignment.
> 
> ~Craig
> 
> >> I have been working on a bug in clang dealing with alignment of wide
> >> strings. Clang is aligning wide string literals to 1 byte boundary.
> >> However, LLVM treats wide strings as 4 byte aligned and generates
> >> memory operations accessing 4 bytes at a time. This works fine on
> >> architectures that allow unaligned access. But for architectures
> >> which don't allow unaligned access, this results in an exception and
> segfault.
> >
> > +      unsigned Align = TI.getWCharAlign() / TI.getCharAlign();
> >
> > This should be dividing by the bit-width of char, not by its
> alignment.
> >
> > Also, there are multiple kinds of wide strings ? official wide
> strings
> > (L), and then UTF-16 (u) and UTF-32 (U) wide strings.  It would be
> > good if this worked for all of them.  You should also make sure that
> > you never
> > *decrease* the alignment of a string, as you might if there were a
> > utf32 string literal followed by a utf16 string literal which
> expanded
> > to the same sequence of bytes.
> >
> > Also, in the future, please give your patch files some sort of
> > sensible file extension, preferably ".txt".
> >
> > John.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110803/31724d74/attachment.txt>