Add support for __attribute__(trivial_abi)

[jingham]

Summary

clang added support for the "trivial_abi" which allows structs that would usually be passed in memory in C++ to be passed in registers instead.

This patch reads in the DWARF attribute that specifies a data type is using this ABI, and marks the Clang type appropriately.

It also adds the decorator goo to skip the test if the compiler doesn't support this attribute.

It also adds a test that tests that we can handle the same struct with and without this attribute in the expression parser and can fetch the return value for it properly. Ironically, the lldb return value code for x86_64 only uses struct size & alignment to tell it how structs would be returned. By that computation this struct should have been returned in registers. So the part of the test handling the non-trivial return is xfailed.

[jingham]

Pavel, I added you mostly because I'm new to this decorator magic and I wanted to have another pair of eyes to make sure I did that right.

[aprantl]

packages/Python/lldbsuite/test/decorators.py

667

Neat!

source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1011

That is reasonable.

[jingham]

packages/Python/lldbsuite/test/decorators.py

667

Credit where it's due: that's an extension of Kuba's trick for checking for the presence of sanitizers.

[jingham]

Checked in as r328389.

[labath]

In D44813#1046379, @jingham wrote:

Pavel, I added you mostly because I'm new to this decorator magic and I wanted to have another pair of eyes to make sure I did that right.

The decorator looks fine. I think most tests use the from lldbsuite.test.decorators import * + @myDecorator pattern because it's shorter, but I don't think that matters.

[labath]

In D44813#1048039, @labath wrote:

In D44813#1046379, @jingham wrote:

Pavel, I added you mostly because I'm new to this decorator magic and I wanted to have another pair of eyes to make sure I did that right.

The decorator looks fine.

Actually, I spoke too soon. The popen invocation was a bit too posix-y for windows to handle. r328489 should fix that.

[llvm-commits]

Thanks for fixing that!

Jim

packages/Python/lldbsuite/test/decorators.py

Arguments:
             for which a test is skipped. None means all architectures.
     """
     return skipTestIfFn(
         _skip_for_android(
             "skipping for android",
             api_levels,
             archs))
 
+def skipUnlessSupportedTypeAttribute(attr):
+    """Decorate the item to skip test unless Clang supports type __attribute__(attr)."""
+    def compiler_doesnt_support_struct_attribute(self):
+        compiler_path = self.getCompiler()
+        compiler = os.path.basename(compiler_path)
+        f = tempfile.NamedTemporaryFile()
+        cmd = "echo 'struct __attribute__((%s)) Test {};' | %s -x c++ -c -o %s - ; exit" % (attr, compiler_path, f.name)
+        p = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True)
+        test_result = p.stderr.read()
+        if attr in test_result:
+            return "Compiler does not support attribute %s"%(attr)
+        return None
+    return skipTestIfFn(compiler_doesnt_support_struct_attribute)
 
 def skipUnlessThreadSanitizer(func):
     """Decorate the item to skip test unless Clang -fsanitize=thread is supported."""
 
     def is_compiler_clang_with_thread_sanitizer(self):
         compiler_path = self.getCompiler()
         compiler = os.path.basename(compiler_path)
         if not compiler.startswith("clang"):

packages/Python/lldbsuite/test/lang/cpp/trivial_abi/Makefile

+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/cpp/trivial_abi/TestTrivialABI.py

+"""
+Test that we work properly with classes with the trivial_abi attribute
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import decorators
+
+class TestTrivialABI(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @decorators.skipUnlessSupportedTypeAttribute("trivial_abi")
+    def test_call_trivial(self):
+        """Test that we can print a variable & call a function with a trivial ABI class."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.cpp")
+        self.expr_test(True)
+
+    @decorators.skipUnlessSupportedTypeAttribute("trivial_abi")
+    @decorators.expectedFailureAll(bugnumber="llvm.org/pr36870")
+    def test_call_nontrivial(self):
+        """Test that we can print a variable & call a function on the same class w/o the trivial ABI marker."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.cpp")
+        self.expr_test(False)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+    def check_value(self, test_var, ivar_value):
+        self.assertTrue(test_var.GetError().Success(), "Invalid valobj: %s"%(test_var.GetError().GetCString()))
+        ivar = test_var.GetChildMemberWithName("ivar")
+        self.assertTrue(test_var.GetError().Success(), "Failed to fetch ivar")
+        self.assertEqual(ivar_value, ivar.GetValueAsSigned(), "Got the right value for ivar")
+        
+    def check_frame(self, thread):
+        frame = thread.frames[0]
+        inVal_var = frame.FindVariable("inVal")
+        self.check_value(inVal_var, 10)
+
+        options = lldb.SBExpressionOptions()
+        inVal_expr = frame.EvaluateExpression("inVal", options)
+        self.check_value(inVal_expr, 10)
+        
+        thread.StepOut()
+        outVal_ret = thread.GetStopReturnValue()
+        self.check_value(outVal_ret, 30)
+        
+    def expr_test(self, trivial):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Set a breakpoint here", self.main_source_file) 
+
+        # Stop in a function that takes a trivial value, and try both frame var & expr to get its value:
+        if trivial:
+            self.check_frame(thread)
+            return
+
+        # Now continue to the same thing without the trivial_abi and see if we get that right:
+        threads = lldbutil.continue_to_breakpoint(process, bkpt)
+        self.assertEqual(len(threads), 1, "Hit my breakpoint the second time.")
+
+        self.check_frame(threads[0])
+        
+

packages/Python/lldbsuite/test/lang/cpp/trivial_abi/main.cpp

+struct __attribute__((trivial_abi)) S_Trivial {
+  ~S_Trivial() {}
+  int ivar = 10;
+};
+
+S_Trivial takeTrivial(S_Trivial inVal)
+{
+  S_Trivial ret_val = inVal;
+  ret_val.ivar = 30;
+  return ret_val;   // Set a breakpoint here
+}
+
+struct S_NotTrivial {
+  ~S_NotTrivial() {}
+  int ivar = 10;
+};
+
+S_NotTrivial takeNotTrivial(S_NotTrivial inVal)
+{
+  S_NotTrivial ret_val = inVal;
+  ret_val.ivar = 30;
+  return ret_val;   // Set a breakpoint here
+}
+
+int
+main()
+{
+  S_Trivial inVal, outVal;
+  outVal = takeTrivial(inVal);
+
+  S_NotTrivial inNotVal, outNotVal;
+  outNotVal = takeNotTrivial(outNotVal);
+
+  return 0; // Set another for return value
+}

source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

if (type_ptr == NULL) {
       case DW_TAG_union_type:
       case DW_TAG_class_type: {
         // Set a bit that lets us know that we are currently parsing this
         dwarf->GetDIEToType()[die.GetDIE()] = DIE_IS_BEING_PARSED;
         bool byte_size_valid = false;
 
         LanguageType class_language = eLanguageTypeUnknown;
         bool is_complete_objc_class = false;
+        size_t calling_convention 
+                = llvm::dwarf::CallingConvention::DW_CC_normal;
+        
         // bool struct_is_class = false;
         const size_t num_attributes = die.GetAttributes(attributes);
         if (num_attributes > 0) {
           uint32_t i;
           for (i = 0; i < num_attributes; ++i) {
             attr = attributes.AttributeAtIndex(i);
             if (attributes.ExtractFormValueAtIndex(i, form_value)) {
               switch (attr) {
if (type_ptr == NULL) {
 
               case DW_AT_APPLE_runtime_class:
                 class_language = (LanguageType)form_value.Signed();
                 break;
 
               case DW_AT_APPLE_objc_complete_type:
                 is_complete_objc_class = form_value.Signed();
                 break;
+              case DW_AT_calling_convention:
+                calling_convention = form_value.Unsigned();
+                break;
                 
               case DW_AT_allocated:
               case DW_AT_associated:
               case DW_AT_data_location:
               case DW_AT_description:
               case DW_AT_start_scope:
               case DW_AT_visibility:
               default:
               case DW_AT_sibling:
if (type_ptr == NULL) {
 
           if (!clang_type_was_created) {
             clang_type_was_created = true;
             clang_type = m_ast.CreateRecordType(decl_ctx, accessibility,
                                                 type_name_cstr, tag_decl_kind,
                                                 class_language, &metadata);
           }
         }
         
         // Store a forward declaration to this class type in case any
         // parameters in any class methods need it for the clang
         // types for function prototypes.
         LinkDeclContextToDIE(m_ast.GetDeclContextForType(clang_type), die);
         type_sp.reset(new Type(die.GetID(), dwarf, type_name_const_str,
                                byte_size, NULL, LLDB_INVALID_UID,
                                Type::eEncodingIsUID, &decl, clang_type,
                                Type::eResolveStateForward));
if (type_ptr == NULL) {
             dwarf->GetForwardDeclDieToClangType()[die.GetDIE()] =
                 clang_type.GetOpaqueQualType();
             dwarf->GetForwardDeclClangTypeToDie()
                 [ClangUtil::RemoveFastQualifiers(clang_type)
                      .GetOpaqueQualType()] = die.GetDIERef();
             m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
           }
         }
+        
+        // If we made a clang type, set the trivial abi if applicable:
+        // We only do this for pass by value - which implies the Trivial ABI.
+        // There isn't a way to assert that something that would normally be
+        // pass by value is pass by reference, so we ignore that attribute if
+        // set.
+        if (calling_convention == llvm::dwarf::DW_CC_pass_by_value) {
+          clang::CXXRecordDecl *record_decl =
+                  m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
+          if (record_decl) {
+            record_decl->setHasTrivialSpecialMemberForCall();
+          }
+        }
+
       } break;
 
       case DW_TAG_enumeration_type: {
         // Set a bit that lets us know that we are currently parsing this
         dwarf->GetDIEToType()[die.GetDIE()] = DIE_IS_BEING_PARSED;
 
         bool is_scoped = false;
         DWARFFormValue encoding_form;