[llvm] r218514 - Revert "Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection."
Duncan P. N. Exon Smith
dexonsmith at apple.com
Fri Sep 26 14:39:36 PDT 2014
> On Sep 26, 2014, at 6:49 AM, Frédéric Riss <friss at apple.com> wrote:
>
>>
>> On 26 Sep 2014, at 14:34, Frederic Riss <friss at apple.com> wrote:
>>
>> Author: friss
>> Date: Fri Sep 26 07:34:06 2014
>> New Revision: 218514
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=218514&view=rev
>> Log:
>> Revert "Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection."
>>
>> This reverts commit r218513.
>>
>> Buildbots using libstdc++ issue an error when trying to copy
>> SmallVector<std::unique_ptr<>>. Revert the commit until we have a fix.
>
> This wasn’t only libstdc++ in fact, MSVC also chocked and I have one libcxx bot that also failed.
>
> David, if you look at the commit, you’ll see that I didn’t replace the SmallVectors we talked about by std::vectors. I tried it, but it failed in a very similar way to the builders. I thought this was some libcxx or SmallVector issue and I was going to report it with a reproducer:
>
> #include <vector>
> #include <memory>
> #include <llvm/ADT/SmallVector.h>
>
> typedef llvm::SmallVector<std::unique_ptr<int>,1> vec_type;
> std::vector<vec_type> vecvec;
>
> void foo() {
> vecvec.push_back(vec_type());
> }
>
> Is this code somehow wrong?
> This fails to compile for me because the move semantics are lost somewhere and the implicitly deleted unique_ptr copy constructor is called. Note that other permutations of vector and SmallVector seem to work.
>
> If I add explicit default and move constructors to DWARFUnitSection, then I am able to replace the SmallVectors by vectors as we discussed, but I don’t understand why this is necessary. Maybe this would also fix the issue seen on the builders, but in either case I can’t see why the compiler generated constructors don’t work.
>
> For the record, here are some builder logs:
> http://lab.llvm.org:8011/builders/clang-native-arm-cortex-a9/builds/21968/steps/compile/logs/stdio
I looked at this one. It looks to me like the compiler hasn’t generated
a move constructor for `DWARFUnitSection`. I knew MSVC had issues like
this, but maybe GCC has trouble to. We can’t really trust compilers to
auto-generate C++11 constructors properly.
Looking at the code, you probably just need something like this:
DwarfUnitSection(DwarfUnitSection &&X)
: SmallVector<std::unique_ptr<UnitType>>(std::move(X)) {}
If MSVC supported it, you could do this instead:
DwarfUnitSection(DwarfUnitSection &&) = default;
> http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/8474/steps/build_Lld/logs/stdio
> http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/13282/steps/build_Lld/logs/stdio
>
> Fred
>
>> Modified:
>> llvm/trunk/lib/DebugInfo/DWARFContext.cpp
>> llvm/trunk/lib/DebugInfo/DWARFContext.h
>>
>> Modified: llvm/trunk/lib/DebugInfo/DWARFContext.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=218514&r1=218513&r2=218514&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/DebugInfo/DWARFContext.cpp (original)
>> +++ llvm/trunk/lib/DebugInfo/DWARFContext.cpp Fri Sep 26 07:34:06 2014
>> @@ -86,17 +86,15 @@ void DWARFContext::dump(raw_ostream &OS,
>>
>> if ((DumpType == DIDT_All || DumpType == DIDT_Types) && getNumTypeUnits()) {
>> OS << "\n.debug_types contents:\n";
>> - for (const auto &TUS : type_unit_sections())
>> - for (const auto &TU : TUS)
>> - TU->dump(OS);
>> + for (const auto &TU : type_units())
>> + TU->dump(OS);
>> }
>>
>> if ((DumpType == DIDT_All || DumpType == DIDT_TypesDwo) &&
>> getNumDWOTypeUnits()) {
>> OS << "\n.debug_types.dwo contents:\n";
>> - for (const auto &DWOTUS : dwo_type_unit_sections())
>> - for (const auto &DWOTU : DWOTUS)
>> - DWOTU->dump(OS);
>> + for (const auto &DWOTU : dwo_type_units())
>> + DWOTU->dump(OS);
>> }
>>
>> if (DumpType == DIDT_All || DumpType == DIDT_Loc) {
>> @@ -339,17 +337,15 @@ void DWARFContext::parseTypeUnits() {
>> uint32_t offset = 0;
>> const DataExtractor &DIData =
>> DataExtractor(I.second.Data, isLittleEndian(), 0);
>> - TUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
>> - auto &TUS = TUs.back();
>> while (DIData.isValidOffset(offset)) {
>> std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
>> getDebugAbbrev(), I.second.Data, getRangeSection(),
>> getStringSection(), StringRef(), getAddrSection(),
>> - &I.second.Relocs, isLittleEndian(), TUS));
>> + &I.second.Relocs, isLittleEndian(), TUs));
>> if (!TU->extract(DIData, &offset))
>> break;
>> - TUS.push_back(std::move(TU));
>> - offset = TUS.back()->getNextUnitOffset();
>> + TUs.push_back(std::move(TU));
>> + offset = TUs.back()->getNextUnitOffset();
>> }
>> }
>> }
>> @@ -380,17 +376,15 @@ void DWARFContext::parseDWOTypeUnits() {
>> uint32_t offset = 0;
>> const DataExtractor &DIData =
>> DataExtractor(I.second.Data, isLittleEndian(), 0);
>> - DWOTUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
>> - auto &TUS = DWOTUs.back();
>> while (DIData.isValidOffset(offset)) {
>> std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
>> getDebugAbbrevDWO(), I.second.Data, getRangeDWOSection(),
>> getStringDWOSection(), getStringOffsetDWOSection(), getAddrSection(),
>> - &I.second.Relocs, isLittleEndian(), TUS));
>> + &I.second.Relocs, isLittleEndian(), DWOTUs));
>> if (!TU->extract(DIData, &offset))
>> break;
>> - TUS.push_back(std::move(TU));
>> - offset = TUS.back()->getNextUnitOffset();
>> + DWOTUs.push_back(std::move(TU));
>> + offset = DWOTUs.back()->getNextUnitOffset();
>> }
>> }
>> }
>>
>> Modified: llvm/trunk/lib/DebugInfo/DWARFContext.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.h?rev=218514&r1=218513&r2=218514&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/DebugInfo/DWARFContext.h (original)
>> +++ llvm/trunk/lib/DebugInfo/DWARFContext.h Fri Sep 26 07:34:06 2014
>> @@ -30,7 +30,7 @@ namespace llvm {
>> class DWARFContext : public DIContext {
>>
>> DWARFUnitSection<DWARFCompileUnit> CUs;
>> - SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> TUs;
>> + DWARFUnitSection<DWARFTypeUnit> TUs;
>> std::unique_ptr<DWARFDebugAbbrev> Abbrev;
>> std::unique_ptr<DWARFDebugLoc> Loc;
>> std::unique_ptr<DWARFDebugAranges> Aranges;
>> @@ -38,7 +38,7 @@ class DWARFContext : public DIContext {
>> std::unique_ptr<DWARFDebugFrame> DebugFrame;
>>
>> DWARFUnitSection<DWARFCompileUnit> DWOCUs;
>> - SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> DWOTUs;
>> + DWARFUnitSection<DWARFTypeUnit> DWOTUs;
>> std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
>> std::unique_ptr<DWARFDebugLocDWO> LocDWO;
>>
>> @@ -77,7 +77,6 @@ public:
>>
>> typedef DWARFUnitSection<DWARFCompileUnit>::iterator_range cu_iterator_range;
>> typedef DWARFUnitSection<DWARFTypeUnit>::iterator_range tu_iterator_range;
>> - typedef iterator_range<SmallVectorImpl<DWARFUnitSection<DWARFTypeUnit>>::iterator> tu_section_iterator_range;
>>
>> /// Get compile units in this context.
>> cu_iterator_range compile_units() {
>> @@ -86,9 +85,9 @@ public:
>> }
>>
>> /// Get type units in this context.
>> - tu_section_iterator_range type_unit_sections() {
>> + tu_iterator_range type_units() {
>> parseTypeUnits();
>> - return tu_section_iterator_range(TUs.begin(), TUs.end());
>> + return tu_iterator_range(TUs.begin(), TUs.end());
>> }
>>
>> /// Get compile units in the DWO context.
>> @@ -98,9 +97,9 @@ public:
>> }
>>
>> /// Get type units in the DWO context.
>> - tu_section_iterator_range dwo_type_unit_sections() {
>> + tu_iterator_range dwo_type_units() {
>> parseDWOTypeUnits();
>> - return tu_section_iterator_range(DWOTUs.begin(), DWOTUs.end());
>> + return tu_iterator_range(DWOTUs.begin(), DWOTUs.end());
>> }
>>
>> /// Get the number of compile units in this context.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list