Page MenuHomePhabricator

shafik (Shafik Yaghmour)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 12 2018, 2:31 PM (114 w, 3 d)

Recent Activity

Thu, Sep 17

shafik updated the diff for D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.

Fix handling of multi-dimensional arrays.

Thu, Sep 17, 1:35 PM

Wed, Sep 16

Herald added a reviewer for D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl: JDevlieghere.

We are going to move forward with this approach (after dealing with the multi-dimensional array case) temporarily. We are seeing crash bugs from this from users and we want to fix it while we explore the solution space more.

Wed, Sep 16, 3:19 PM

Sun, Sep 13

shafik added inline comments to D87441: Speedup collecting DWARF attribute values.
Sun, Sep 13, 9:43 PM · Restricted Project

Tue, Sep 8

shafik accepted D87289: [lldb] Don't infinite loop in SemaSourceWithPriorities::CompleteType when trying to complete a forward decl.

LGTM

Tue, Sep 8, 1:46 PM · Restricted Project

Thu, Sep 3

shafik updated subscribers of D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.

@martong I have been experimenting w/ how we pass around ImportDefinitionKind and pushing it through to more places does not help. I also tried defaulting most locations to IDK_Everything to experiment to see if maybe I was missing some crucial point and no luck. So while it seemed like a good direction it has not worked out, unless I missed something in your idea.

Thu, Sep 3, 1:41 PM

Thu, Aug 27

shafik added inline comments to D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.
Thu, Aug 27, 2:19 PM

Wed, Aug 26

shafik accepted D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes.

LGTM with minor comments. Thank you for these fixes, they are awesome!

Wed, Aug 26, 3:12 PM · Restricted Project
shafik requested review of D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.
Wed, Aug 26, 1:53 PM

Mon, Aug 24

shafik committed rG93b255142bb7: [LLDB] Fix how ValueObjectVariable handles DW_AT_const_value when the… (authored by shafik).
[LLDB] Fix how ValueObjectVariable handles DW_AT_const_value when the…
Mon, Aug 24, 3:17 PM
shafik closed D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.
Mon, Aug 24, 3:17 PM · Restricted Project
shafik added inline comments to D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.
Mon, Aug 24, 2:14 PM · Restricted Project
shafik updated the diff for D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.

-Add REQUIRES x86

  • Fix-up the comments in the test
Mon, Aug 24, 1:42 PM · Restricted Project
shafik added inline comments to D86436: [lldb] Fix Type::GetByteSize for pointer types.
Mon, Aug 24, 10:18 AM · Restricted Project
shafik added inline comments to D86436: [lldb] Fix Type::GetByteSize for pointer types.
Mon, Aug 24, 10:06 AM · Restricted Project

Aug 21 2020

shafik added inline comments to D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.
Aug 21 2020, 6:47 AM · Restricted Project
shafik added a comment to D86348: [lldb/DWARF] More DW_AT_const_value fixes.

Thank you finding these additional fixed, it looks good but I don't know some of the details as well as Adrian so I would prefer he gives the LGTM.

Aug 21 2020, 6:43 AM · Restricted Project

Aug 20 2020

shafik requested review of D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.
Aug 20 2020, 11:22 AM · Restricted Project

Aug 18 2020

shafik accepted D86140: [lldb] Add typedefs to the DeclContext they are created in.

This is nice catch! LGTM

Aug 18 2020, 10:11 AM · Restricted Project

Aug 13 2020

shafik accepted D85904: [lldb] Check Decl kind when completing -flimit-debug-info types.

Minor comment but LGTM, nice fix!

Aug 13 2020, 2:07 PM · Restricted Project
shafik committed rG25bbceb047a3: [LLDB] Fix how ValueObjectChild handles bit-fields stored in a Scalar in… (authored by shafik).
[LLDB] Fix how ValueObjectChild handles bit-fields stored in a Scalar in…
Aug 13 2020, 11:53 AM
shafik closed D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
Aug 13 2020, 11:53 AM · Restricted Project
shafik accepted D85648: [lldb] Fix a crash when the ASTImporter is giving us two Imported callbacks for the same target decl.

LGTM, it is sad we lose the deterministic ordering though. Would it be efficient enough to use a vector but keep the set to prevent duplicates?

Aug 13 2020, 11:42 AM · Restricted Project
shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

Update test name

Aug 13 2020, 10:31 AM · Restricted Project

Aug 12 2020

shafik added a comment to D81471: [lldb] Add support for using integral const static data members in the expression evaluator.

I am curious about constexpr static as well here.

Aug 12 2020, 9:20 PM
shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

Updated test using more compact code from Fred.

Aug 12 2020, 6:49 PM · Restricted Project
shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

Replacing python test with Shell test

Aug 12 2020, 1:47 PM · Restricted Project

Aug 11 2020

shafik added a comment to D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

We found a way to hand modify the assembly and it looks good, I just need to convert it to a test.

Aug 11 2020, 8:40 PM · Restricted Project
shafik added a comment to D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

This manifested itself for variables in registers because those end up being described as eValueTypeScalar, is that so?

If that's the case, then I think this would also manifest for variables described via DW_OP_constu 0xdead, DW_OP_stack_value. And if *that* is true, then this could be tested a lot simpler by having a global variable described by DW_OP_stack_value and "target variable"-ing it -- no makefiles, no python, just a simple .s file. And it would run on all platforms, not just darwin.

test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s tests a very similar thing in precisely this way. I suspect you could just take that test as a template and replace the struct definition with one that contains bitfieds.

Aug 11 2020, 2:49 PM · Restricted Project

Aug 10 2020

shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

Updated to use llvm.org clang, remove header files from the main.c and add the commit hash for the clang so that it is easier to replicate the main.s in the future.

Aug 10 2020, 2:49 PM · Restricted Project

Aug 6 2020

shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
  • Add more tests
Aug 6 2020, 12:26 PM · Restricted Project
shafik added inline comments to D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
Aug 6 2020, 12:21 PM · Restricted Project
shafik added inline comments to D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
Aug 6 2020, 11:36 AM · Restricted Project
shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
  • Refactored the Makefile and test based on offline comments from Adrian.
Aug 6 2020, 10:41 AM · Restricted Project
shafik updated the diff for D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

Removing use of -O1 from Makefile.

Aug 6 2020, 9:59 AM · Restricted Project
shafik updated subscribers of D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
Aug 6 2020, 9:58 AM · Restricted Project

Aug 5 2020

shafik added a comment to D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .

Note: for the test I did not want to rely on clang choosing to pass the union by register, so I used assembly which ensures I will obtain the behavior I am looking to capture for the test.

Aug 5 2020, 5:02 PM · Restricted Project
shafik requested review of D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue() .
Aug 5 2020, 4:59 PM · Restricted Project

Aug 3 2020

shafik added a comment to D85141: [lldb] Enable std::pair in CxxModuleHandler.

Do we understand how to fix the crash in the case where we get a pair back? I am not sure how frequent the use case is.

Aug 3 2020, 10:25 AM · Restricted Project, Restricted Project

Jul 29 2020

shafik committed rG6700f4b9fe63: [LLDB] Add checks for ValueObjectSP in Cocoa summary providers (authored by shafik).
[LLDB] Add checks for ValueObjectSP in Cocoa summary providers
Jul 29 2020, 2:47 PM
shafik closed D84272: Add checks for ValueObjectSP in Cocoa summary providers.
Jul 29 2020, 2:47 PM · Restricted Project
shafik updated the summary of D84272: Add checks for ValueObjectSP in Cocoa summary providers.
Jul 29 2020, 1:26 PM · Restricted Project

Jul 24 2020

shafik committed rG0db2934b0fa9: [ASTImporter] Modify ImportDefiniton for ObjCInterfaceDecl so that we always… (authored by shafik).
[ASTImporter] Modify ImportDefiniton for ObjCInterfaceDecl so that we always…
Jul 24 2020, 1:15 PM
shafik closed D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition.
Jul 24 2020, 1:15 PM · Restricted Project, Restricted Project

Jul 23 2020

shafik updated the diff for D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition.

Updating diff since the parent landed.

Jul 23 2020, 4:42 PM · Restricted Project, Restricted Project
shafik added inline comments to D84440: [llb] Remove the user-defined copy-ctor in ConstString.
Jul 23 2020, 11:50 AM · Restricted Project
shafik accepted D84285: Unify the return value of GetByteSize to an llvm::Optional<uint64_t> (NFC-ish).

LGTM

Jul 23 2020, 10:28 AM · Restricted Project
shafik added a comment to D84272: Add checks for ValueObjectSP in Cocoa summary providers.

I updated to include the radar number.

Jul 23 2020, 10:17 AM · Restricted Project
shafik updated the summary of D84272: Add checks for ValueObjectSP in Cocoa summary providers.
Jul 23 2020, 10:10 AM · Restricted Project

Jul 22 2020

shafik added a comment to D84272: Add checks for ValueObjectSP in Cocoa summary providers.

why? Do you have a testcase?

Jul 22 2020, 8:35 PM · Restricted Project
shafik updated the diff for D84272: Add checks for ValueObjectSP in Cocoa summary providers.

Removing text->GetValueAsUnsigned(0) == 0 check.

Jul 22 2020, 4:25 PM · Restricted Project
shafik added a comment to D84272: Add checks for ValueObjectSP in Cocoa summary providers.

I was testing out how NSStringSummaryProvider deals w/ NULL using NSString *foo = nullptr and we filter out NULL values in ValueObjectPrinter::GetValueSummaryError(...) :

Jul 22 2020, 4:00 PM · Restricted Project

Jul 21 2020

shafik created D84272: Add checks for ValueObjectSP in Cocoa summary providers.
Jul 21 2020, 3:21 PM · Restricted Project
shafik added inline comments to D84267: Thread ExecutionContextScope through GetByteSize where possible (NFC-ish).
Jul 21 2020, 2:12 PM · Restricted Project
shafik added inline comments to D84267: Thread ExecutionContextScope through GetByteSize where possible (NFC-ish).
Jul 21 2020, 2:05 PM · Restricted Project

Jul 20 2020

shafik committed rGa54c42df9a72: Fix how we handle bit-fields for Objective-C when creating an AST (authored by shafik).
Fix how we handle bit-fields for Objective-C when creating an AST
Jul 20 2020, 9:19 PM
shafik closed D83433: Fix how we handle bit-fields for Objective-C when creating an AST.
Jul 20 2020, 9:18 PM · Restricted Project
shafik added a comment to D84015: [lldb] Remove orphaned modules in a loop.

Thank you for this fix!

Jul 20 2020, 10:20 AM · Restricted Project

Jul 16 2020

shafik added a comment to D83970: [ASTImporter] Refactor ASTImporter to support custom downstream tests.

LGTM but I want @martong to take a look as well and accept.

Jul 16 2020, 11:50 AM · Restricted Project
shafik created D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition.
Jul 16 2020, 11:15 AM · Restricted Project, Restricted Project

Jul 15 2020

shafik added a comment to D83840: [lldb][test] Prevent infinite loop while looking for use_lldb_suite_root.py..

@labath why do we need two copies of use_lldb_suite.py?

Jul 15 2020, 1:29 PM · Restricted Project

Jul 14 2020

shafik updated the diff for D83433: Fix how we handle bit-fields for Objective-C when creating an AST.

Added clarifying comment.

Jul 14 2020, 5:05 PM · Restricted Project
shafik updated the summary of D83433: Fix how we handle bit-fields for Objective-C when creating an AST.
Jul 14 2020, 4:57 PM · Restricted Project
shafik committed rGe77442fa315b: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...) (authored by shafik).
[ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)
Jul 14 2020, 4:46 PM
GitHub <noreply@github.com> committed rG01397d1d64b1: Merge pull request #1080 from shafik/56671052_fix_covariant_return_type_handling (authored by shafik).
Merge pull request #1080 from shafik/56671052_fix_covariant_return_type_handling
Jul 14 2020, 4:43 PM
shafik committed rG58d95fb2f550: [lldb] Fix another crash in covariant type handling (authored by labath).
[lldb] Fix another crash in covariant type handling
Jul 14 2020, 4:43 PM
shafik committed rGba8cca0196bb: [lldb] Complete return types of CXXMethodDecls to prevent crashing due to… (authored by teemperor).
[lldb] Complete return types of CXXMethodDecls to prevent crashing due to…
Jul 14 2020, 4:43 PM
shafik committed rGfbec4e225071: [LLDB] Initialize temporary token (authored by bkramer).
[LLDB] Initialize temporary token
Jul 14 2020, 4:38 PM
shafik committed rG564d7067f9d2: [LLDB] CPlusPlusNameParser does not handles templated operator< properly (authored by shafik).
[LLDB] CPlusPlusNameParser does not handles templated operator< properly
Jul 14 2020, 4:38 PM
shafik committed rG455405571821: [LLDB] Fix handling of bit-fields when there is a base class when parsing DWARF (authored by shafik).
[LLDB] Fix handling of bit-fields when there is a base class when parsing DWARF
Jul 14 2020, 4:38 PM
GitHub <noreply@github.com> committed rG2c1c07003165: Merge pull request #964 from shafik/43399538_formatter_for_libcpp_std_unique_ptr (authored by shafik).
Merge pull request #964 from shafik/43399538_formatter_for_libcpp_std_unique_ptr
Jul 14 2020, 4:36 PM
shafik committed rG607fbb56b296: [DataFormatters] Add formatter for libc++ std::unique_ptr (authored by shafik).
[DataFormatters] Add formatter for libc++ std::unique_ptr
Jul 14 2020, 4:36 PM
GitHub <noreply@github.com> committed rGafd54d47d7ea: Merge pull request #931 from shafik/60173832_remove_temp_params_from_functionte… (authored by shafik).
Merge pull request #931 from shafik/60173832_remove_temp_params_from_functionte…
Jul 14 2020, 4:35 PM
shafik committed rG6cdbe04d7d68: [lldb] Remove template parameters from FunctionTemplateDecl names (authored by shafik).
[lldb] Remove template parameters from FunctionTemplateDecl names
Jul 14 2020, 4:35 PM
GitHub <noreply@github.com> committed rG837fba4840ee: Merge pull request #879 from shafik/60163218_fix_dsymutil_to_strip_template_par… (authored by shafik).
Merge pull request #879 from shafik/60163218_fix_dsymutil_to_strip_template_par…
Jul 14 2020, 4:32 PM
shafik committed rGeb4c882b1803: [dsymutil] Fix template stripping in getDIENames(...) to account for overloaded… (authored by shafik).
[dsymutil] Fix template stripping in getDIENames(...) to account for overloaded…
Jul 14 2020, 4:32 PM
GitHub <noreply@github.com> committed rGcd53dcf23198: Merge pull request #665 from shafik/54812864_fix_unnamed_bitfield_parse_dwarf (authored by shafik).
Merge pull request #665 from shafik/54812864_fix_unnamed_bitfield_parse_dwarf
Jul 14 2020, 4:18 PM
shafik committed rGcd470d7cc890: [LLDB] Fix the handling of unnamed bit-fields when parsing DWARF (authored by shafik).
[LLDB] Fix the handling of unnamed bit-fields when parsing DWARF
Jul 14 2020, 4:18 PM
shafik committed rG6ee04b9df14b: [lldb] Fix nondeterminism in TestCppBitfields (authored by labath).
[lldb] Fix nondeterminism in TestCppBitfields
Jul 14 2020, 4:18 PM
shafik added a comment to D83796: [ObjC] Wrap namespace-global structure in an anonymous namespace to avoid ODR violations.

It does indeed solve the immediate problem but if those namespaces aren't really meant to be shared across translation units they should have different names but that is for another PR.

Jul 14 2020, 2:21 PM · Restricted Project
shafik added a comment to D83787: [lldb/Test] Always set the cleanupSubprocesses tear down hook.

Curious, why didn't we do it this way before?

Jul 14 2020, 10:28 AM · Restricted Project

Jul 9 2020

shafik added a comment to D83433: Fix how we handle bit-fields for Objective-C when creating an AST.

@aprantl I think this Objective-C Runtime Programming Guide: Bye Encodings entry and this sample program answer the rest of your questions:

Jul 9 2020, 2:38 PM · Restricted Project
shafik updated the summary of D83433: Fix how we handle bit-fields for Objective-C when creating an AST.
Jul 9 2020, 1:08 PM · Restricted Project
shafik updated the summary of D83433: Fix how we handle bit-fields for Objective-C when creating an AST.
Jul 9 2020, 1:04 PM · Restricted Project
shafik added a comment to D83433: Fix how we handle bit-fields for Objective-C when creating an AST.

We can never know the offsets statically b/c of how Objective-C deals with the fragile base class problem the runtime may have to shift fields over and we can not calculate this statically.

Jul 9 2020, 1:03 PM · Restricted Project
shafik added inline comments to D83433: Fix how we handle bit-fields for Objective-C when creating an AST.
Jul 9 2020, 9:53 AM · Restricted Project

Jul 8 2020

shafik created D83433: Fix how we handle bit-fields for Objective-C when creating an AST.
Jul 8 2020, 3:03 PM · Restricted Project
shafik committed rG63b0f8c788d8: [RecordLayout] Fix ItaniumRecordLayoutBuilder so that is grabs the correct… (authored by shafik).
[RecordLayout] Fix ItaniumRecordLayoutBuilder so that is grabs the correct…
Jul 8 2020, 10:07 AM
shafik closed D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.
Jul 8 2020, 10:07 AM · Restricted Project, Restricted Project
shafik updated the diff for D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.

Moved from shell test

Jul 8 2020, 6:38 AM · Restricted Project, Restricted Project
shafik added inline comments to D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.
Jul 8 2020, 6:17 AM · Restricted Project, Restricted Project

Jul 7 2020

shafik updated the diff for D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.
  • Removing spurious local variables in test
  • Simplifying the test
Jul 7 2020, 3:04 PM · Restricted Project, Restricted Project
shafik added a comment to D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.

I think we are talking about different things. My question was why is this a 'Shell' test (like, a test in the Shell directory that uses FileCheck) and not a test in the API directory (using Python). An 'API' test could use the proper expression testing tools. And it could actually run when doing on-device testing (which is to my knowledge not supported for Shell tests) which seems important for a test concerning a bug that only triggers when doing on-device testing (beside that one ubuntu ARM bot).

Jul 7 2020, 9:53 AM · Restricted Project, Restricted Project

Jul 6 2020

shafik added a comment to D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.

Thanks for tracking this down, this is a really nasty bug...

The fix itself is obviously fine, but I think I'm out of the loop regarding the testing strategy. We talked about adding a Clang test for this with the help of this layout overwrite JSON file. I assume that extending this to cover virtual bases turned out to be more complicated than expected? FWIW, I'm not necessarily the biggest fan of this Clang test option so I would be fine if we leave it as-is.

I think having an LLDB test is a good idea, but it's not clear why it's a Shell test. If I understand correctly this test requires running on arm64 (so, a remote test target), but from what I remember all the remote platform support is only in dotest.py? Also pretty much all other expression evaluation tests and the utilities for that are in the Python/API test infrastructure, so it's a bit out of place.

Also I think the test can be in general much simpler than an arm64-specific test. We get all base class offsets wrong in LLDB, so we can just write a simple test where you change the layout of some structs in a way that it doesn't fit the default layout. E.g., just putting alignas on a base class and then reading fields should be enough to trigger the same bug.

Jul 6 2020, 5:07 PM · Restricted Project, Restricted Project
shafik updated the diff for D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.

Adding a second test that is not arm64 specific.

Jul 6 2020, 5:04 PM · Restricted Project, Restricted Project
shafik accepted D83199: [lldb/DWARF] Add a utility function for (forceful) completion of types.

LGTM

Jul 6 2020, 3:06 PM · Restricted Project
shafik added inline comments to D83256: [lldb] Fix unaligned load in DataExtractor.
Jul 6 2020, 2:07 PM · Restricted Project
shafik added inline comments to D83256: [lldb] Fix unaligned load in DataExtractor.
Jul 6 2020, 2:03 PM · Restricted Project

Jul 2 2020

shafik accepted D75740: [ASTImporter] Corrected import of repeated friend declarations..

LGTM

Jul 2 2020, 2:03 PM · Restricted Project
shafik added a comment to D82881: [DEBUGINFO]Fix debug info for packed bitfields..

So with the debug info before this fix what behavior does gdb and lldb have?

Jul 2 2020, 10:47 AM · debug-info, Restricted Project

Jul 1 2020

shafik created D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source.
Jul 1 2020, 4:13 PM · Restricted Project, Restricted Project