<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 14, 2014 at 12:03 AM, Simon Atanasyan <span dir="ltr"><<a href="mailto:simon@atanasyan.com" target="_blank">simon@atanasyan.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Nov 14, 2014 at 10:24 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> On Thu, Nov 13, 2014 at 11:15 PM, Simon Atanasyan <<a href="mailto:simon@atanasyan.com">simon@atanasyan.com</a>><br>
> wrote:<br>
<br>
</span>[...]<br>
<span class=""><br>
>> ==============================================================================<br>
>> --- lld/trunk/lib/Core/SymbolTable.cpp (original)<br>
>> +++ lld/trunk/lib/Core/SymbolTable.cpp Fri Nov 14 01:15:43 2014<br>
>> @@ -187,9 +187,10 @@ bool SymbolTable::addByName(const Atom &<br>
>>    case NCR_Second:<br>
>>      useNew = true;<br>
>>      break;<br>
>> -  case NCR_DupDef:<br>
>> -    switch (mergeSelect(cast<DefinedAtom>(existing)->merge(),<br>
>> -                        cast<DefinedAtom>(&newAtom)->merge())) {<br>
>> +  case NCR_DupDef: {<br>
>> +    const auto *existingDef = cast<DefinedAtom>(existing);<br>
>> +    const auto *newDef = cast<DefinedAtom>(&newAtom);<br>
><br>
><br>
> These casts should already be safe & could remain as they were before, I<br>
> believe. the llvm::cast machinery preserves const without having to specify<br>
> it explicitly.<br>
<br>
</span>You are right, the llvm::cast machinery preserves const. But I prefer<br>
to use the const qualifier with the auto keyword to emphasize the<br>
constness of a variable. Besides that, the same pattern is used a few<br>
lines below in the NCR_DupUndef case.<br></blockquote><div><br>I hope you could do that as cast<const T>(...)->... if you want to be explicit about the const (if that doesn't work - we'll fix the bug in the llvm::cast machinery) - but if you prefer the named variables, that's cool too.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
[...]<br>
<span class=""><br>
>> ==============================================================================<br>
>> --- lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h (original)<br>
>> +++ lld/trunk/lib/ReaderWriter/ELF/Mips/MipsELFFile.h Fri Nov 14 01:15:43<br>
>> 2014<br>
>> @@ -216,9 +216,9 @@ private:<br>
>>      case llvm::ELF::R_MIPS_32:<br>
>>      case llvm::ELF::R_MIPS_GPREL32:<br>
>>      case llvm::ELF::R_MIPS_PC32:<br>
>> -      return *(int32_t *)ap;<br>
>> +      return *(const int32_t *)ap;<br>
><br>
><br>
> This and all casts like it are likely UB (if the underlying pointer is just<br>
> pointing to bytes, not an actual int32_t object) and we should be using our<br>
> utility functions (I forget where they are or what they're called) that can<br>
> read from bytes into ints, etc (using memcpy, the blessed way to do this in<br>
> a well defined manner).<br>
<br>
</span>Yeah, it is in my TODO list. I plan to clean up that code when start<br>
to implement big-endian target support.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Simon Atanasyan<br>
</font></span></blockquote></div><br></div></div>