Page MenuHomePhabricator

[lldb][DWARF] Infer no_unique_address attribute
Needs ReviewPublic

Authored by kpdev42 on Feb 5 2023, 11:53 AM.

Details

Summary

The no_unique_address attribute is currently a part of C++20 standard and is being used in both libstdc++ and libc++. This causes problems when debugging C++ code with lldb because the latter doesn't expect two non-empty base classes having the same offset and crashes with assertion. Patch attempts to infer this attribute by detecting two consecutive base classes with the same offset to derived class and marking first of those classes as empty and adding no_unique_address attribute to all of its fields.

Diff Detail

Event Timeline

kpdev42 created this revision.Feb 5 2023, 11:53 AM
Herald added a reviewer: shafik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
kpdev42 requested review of this revision.Feb 5 2023, 11:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2023, 11:53 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Michael137 added a comment.EditedFeb 5 2023, 5:48 PM

From a first glance looks ok but ideally the clang changes would be separate reviews. E.g.,

  1. Adding markEmpty()
  2. ASTImporter change with unit-test
  3. LLDB changes
clayborg edited reviewers, added: JDevlieghere, jingham; removed: k8stone.Feb 6 2023, 2:37 PM

Changes look fine to me. I would like someone that specializes in the expression parser to give the final ok though.

lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
551

revert whitespace only changes

balazske added inline comments.
clang/lib/AST/ASTImporter.cpp
3896

The import of attributes is handled in function ASTImporter::Import(Decl*). This new line will probably copy all attributes, that may not work in all cases dependent on the attribute types. This may interfere with the later import of attributes, probably these will be duplicated. What was the need for this line? (Simple attributes that do not have references to other nodes could be copied at this place.)

kpdev42 added inline comments.Mon, Feb 20, 8:43 PM
clang/lib/AST/ASTImporter.cpp
3896

Unfortunately it is too late to copy attribute in ASTImporter::Import(Decl*), because field has already been added to record in a call to ImportImpl (VisitFieldDecl/addDeclInternal). I've reused the current way of cloning attributes in VisitFieldDecl.