[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

[George Rimar via Phabricator via lldb-commits]

grimar updated this revision to Diff 169177.
grimar marked 4 inline comments as done.
grimar added a comment.

- Addressed review comments.


https://reviews.llvm.org/D52689

Files:
  lit/Breakpoint/Inputs/implicit_const_form_support.yaml
  lit/Breakpoint/implicit_const_form_support.test
  source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h

Index: source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -60,8 +60,12 @@
   const DWARFUnit *GetCompileUnit() const { return m_cu; }
   void SetCompileUnit(const DWARFUnit *cu) { m_cu = cu; }
   dw_form_t Form() const { return m_form; }
+  dw_form_t& FormRef() { return m_form; }
   void SetForm(dw_form_t form) { m_form = form; }
   const ValueType &Value() const { return m_value; }
+  ValueType &ValueRef() { return m_value; }
+  void SetValue(const ValueType &val) { m_value = val; }
+
   void Dump(lldb_private::Stream &s) const;
   bool ExtractValue(const lldb_private::DWARFDataExtractor &data,
 lldb::offset_t *offset_ptr);
Index: source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -165,6 +165,9 @@
 
 bool DWARFFormValue::ExtractValue(const DWARFDataExtractor &data,
   lldb::offset_t *offset_ptr) {
+  if (m_form == DW_FORM_implicit_const)
+return true;
+
   bool indirect = false;
   bool is_block = false;
   m_value.data = NULL;
@@ -366,6 +369,7 @@
 
   // 0 bytes values (implied from DW_FORM)
   case DW_FORM_flag_present:
+  case DW_FORM_implicit_const:
 return true;
 
 // 1 byte values
@@ -822,6 +826,7 @@
 case DW_FORM_ref_sig8:
 case DW_FORM_GNU_str_index:
 case DW_FORM_GNU_addr_index:
+case DW_FORM_implicit_const:
   return true;
 default:
   break;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -178,7 +178,7 @@
   DumpAttribute(SymbolFileDWARF *dwarf2Data, const DWARFUnit *cu,
 const lldb_private::DWARFDataExtractor &debug_info_data,
 lldb::offset_t *offset_ptr, lldb_private::Stream &s,
-dw_attr_t attr, dw_form_t form);
+dw_attr_t attr, DWARFFormValue &form_value);
   // This one dumps the comp unit name, objfile name and die offset for this die
   // so the stream S.
   void DumpLocation(SymbolFileDWARF *dwarf2Data, DWARFUnit *cu,
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -174,6 +174,10 @@
   debug_info_data.GetU32(&offset);
 break;
 
+  case DW_FORM_implicit_const:
+form_size = 0;
+break;
+
   default:
 *offset_ptr = m_offset;
 return false;
@@ -233,15 +237,16 @@
 
 // Skip all data in the .debug_info for the attributes
 const uint32_t numAttributes = abbrevDecl->NumAttributes();
-uint32_t i;
-dw_attr_t attr;
-dw_form_t form;
-for (i = 0; i < numAttributes; ++i) {
-  abbrevDecl->GetAttrAndFormByIndexUnchecked(i, attr, form);
+for (uint32_t i = 0; i < numAttributes; ++i) {
+  DWARFFormValue form_value;
+  form_value.SetCompileUnit(cu);
+
+  dw_attr_t attr;
+  abbrevDecl->GetAttrAndFormValueByIndex(i, attr, form_value);
+  dw_form_t form = form_value.Form();
 
   if (isCompileUnitTag &&
   ((attr == DW_AT_entry_pc) || (attr == DW_AT_low_pc))) {
-DWARFFormValue form_value(cu, form);
 if (form_value.ExtractValue(debug_info_data, &offset)) {
   if (attr == DW_AT_low_pc || attr == DW_AT_entry_pc)
 const_cast(cu)->SetBaseAddress(
@@ -287,6 +292,7 @@
 
   // 0 sized form
   case DW_FORM_flag_present:
+  case DW_FORM_implicit_const:
 form_size = 0;
 break;
 
@@ -417,14 +423,15 @@
   return false;
 
 const uint32_t numAttributes = abbrevDe

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

[George Rimar via Phabricator via lldb-commits]

grimar added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:23
+  DWARFAttribute(dw_attr_t attr, dw_form_t form,
+ DWARFFormValue::ValueType value)
+  : m_attr(attr), m_form(form), m_value(value) {}

clayborg wrote:
> Do we need to use a "DWARFFormValue::ValueType" here? Right now we only need 
> a int64_t and DWARFFormValue::ValueType is larger than that. It does future 
> proof us a bit and there aren't all that many abbreviations, even in a large 
> DWARF file. Just thinking out loud here
My thoughts to use `DWARFFormValue::ValueType` were that it seems a bit more 
generic,
as standard can add other kinds of values rather than `int64_t` perhaps in 
future.
And it seems to be a bit cleaner because `DWARFFormValue::ValueType` is already
existent class representing the form value while `int64_t` would be kind of 
exception/special case.
I agree that for now, it is a bit excessive to have `DWARFFormValue::ValueType` 
here though. 



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:630-631
 
   DumpAttribute(dwarf2Data, cu, debug_info_data, &offset, s, attr,
-form);
+form_value.Form());
 }

clayborg wrote:
> DumpAttribute will need to take the full form_value in order to dump 
> DW_FORM_implicit_const forms correctly. Change DumpAttribute to take a 
> "form_value"
Done. I had to make it non-const `DWARFFormValue &form_value` because 
`ExtractValue` call inside `DumpAttribute` is not const.
As an alternative, we could probably use pass it by value here. But since there 
is a only one `DumpAttribute` call, I think its ok.


https://reviews.llvm.org/D52689



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Thanks a lot for so detailed answer, it helps!

So we need to parse a FPO program and to convert it in a DWARF expression too. 
The problem here (in the DIA case) is that I don't know how to retrieve the 
required FPO range (we have a symbol context when creating a variable, but it 
seems that it doesn't contain enough information). As for the raw PDB case, can 
the same `S_LOCAL` have several `S_DEFRANGE_FRAMEPOINTER_REL` records for 
several ranges? If so, then the problem exists for the raw PDB case too, but 
there's a solution: we can combine processing of all 
`S_DEFRANGE_FRAMEPOINTER_REL` records in the single DWARF expression (and call 
the required FPO program depending on current `eip`).

The interesting moment here is that MSVC doesn't emit correct information for 
locals. For the program `aaa.cpp`:

  void bar(char a, short b, int c) { }
  
  void __fastcall foo(short arg_0, float arg_1) {
char loc_0 = 'x';
double __declspec(align(8)) loc_1 = 0.5678;
bar(1, 2, 3);
  }
  
  int main(int argc, char *argv[]) {
foo(, 0.1234);
return 0;
  }

compiled with `cl /Zi /Oy aaa.cpp` we have the next disassembly of `foo`:

  pushebp
  mov ebp, esp
  and esp, 0FFF8h
  sub esp, 10h
  mov [esp+4], cx ; arg_0
  mov byte ptr [esp+3], 'x' ; loc_0
  movsd   xmm0, ds:__real@3fe22b6ae7d566cf
  movsd   qword ptr [esp+8], xmm0 ; loc_1
  push3   ; c
  push2   ; b
  push1   ; a
  callj_?bar@@YAXDFH@Z ; bar(char,short,int)
  add esp, 0Ch
  mov esp, ebp
  pop ebp
  retn4

but MSVC emits the next info about locals:

  296 | S_GPROC32 [size = 44] `foo`
parent = 0, end = 452, addr = 0001:23616, code size = 53
type = `0x1003 (void (short, float))`, debug start = 14, debug end = 
47, flags = has fp
  340 | S_FRAMEPROC [size = 32]
size = 16, padding size = 0, offset to padding = 0
bytes of callee saved registers = 0, exception handler addr = :
local fp reg = VFRAME, param fp reg = EBP
flags = has async eh | opt speed
  372 | S_REGREL32 [size = 20] `arg_0`
type = 0x0011 (short), register = ESP, offset = 4294967284
  392 | S_REGREL32 [size = 20] `arg_1`
type = 0x0040 (float), register = EBP, offset = 8
  412 | S_REGREL32 [size = 20] `loc_1`
type = 0x0041 (double), register = ESP, offset = 4294967288
  432 | S_REGREL32 [size = 20] `loc_0`
type = 0x0070 (char), register = ESP, offset = 4294967283
  452 | S_END [size = 4]

so neither LLDB nor Visual Studio show valid values (except for `arg_1`).

In https://reviews.llvm.org/D53086#1260787, @zturner wrote:

> You didn't include it here, but I notice the test file just writes `clang-cl 
> /Zi`.  Should we be passing `-m32` or `-m64`?  Currently, the test just runs 
> with whatever architecture happens to be set via the VS command prompt.  The 
> behavior here is different on x86 and x64 so perhaps it requires separate 
> tests.


The problem is that x64 LLDB can't debug x86 applications on Windows (and vice 
versa), so I rely here on the fact that tests are run in the same VS command 
prompt as LLVM was built. But yes, it's still possible to run tests for x86 
LLVM under the x64 VS command prompt, so this one will fail. I think we can 
somehow guard this when I'll implement `compiler_config` you have mentioned in 
https://reviews.llvm.org/D52618.

In https://reviews.llvm.org/D53086#1260685, @stella.stamenova wrote:

> Could you document that in the test?


Sure, thanks!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53086



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov updated this revision to Diff 169181.

https://reviews.llvm.org/D53086

Files:
  lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
  lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script


Index: lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
===
--- lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
+++ lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
@@ -1,4 +1,4 @@
-breakpoint set --file VariablesLocationsTest.cpp --line 6
+breakpoint set --file VariablesLocationsTest.cpp --line 8
 
 run
 
Index: lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
===
--- lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
+++ lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
@@ -2,7 +2,9 @@
 
 void __fastcall foo(short arg_0, float arg_1) {
   char loc_0 = 'x';
-  double loc_1 = 0.5678;
+  // TODO: make it double when FPO support in `PDBLocationToDWARFExpression`
+  // will be implemented
+  float loc_1 = 0.5678;
 }
 
 int main(int argc, char *argv[]) {


Index: lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
===
--- lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
+++ lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.script
@@ -1,4 +1,4 @@
-breakpoint set --file VariablesLocationsTest.cpp --line 6
+breakpoint set --file VariablesLocationsTest.cpp --line 8
 
 run
 
Index: lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
===
--- lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
+++ lit/SymbolFile/PDB/Inputs/VariablesLocationsTest.cpp
@@ -2,7 +2,9 @@
 
 void __fastcall foo(short arg_0, float arg_1) {
   char loc_0 = 'x';
-  double loc_1 = 0.5678;
+  // TODO: make it double when FPO support in `PDBLocationToDWARFExpression`
+  // will be implemented
+  float loc_1 = 0.5678;
 }
 
 int main(int argc, char *argv[]) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added subscribers: zturner, jasonmolenda, labath.
aleksandr.urakov added a comment.

As for aligned stack cross-platform problems, I mean also problems with stack 
unwinding. They seem to appear on non-Windows too. It's because 
`x86AssemblyInspectionEngine` doesn't support stack alignment now. I've made 
some changes locally to fix it, but they are still raw to publish. The main 
idea is to save one more frame address (along with CFA) for every row of an 
unwind plan (I've called this AFA - aligned frame address), and add an analysis 
for `and esp, ...` etc. to `x86AssemblyInspectionEngine`. What do you think 
about a such approach?


https://reviews.llvm.org/D53086



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53092: [lldb] Add support in Status::AsCString to retrieve win32 system error strings

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Otherwise LGTM!




Comment at: unittests/Utility/StatusTest.cpp:68-74
+  EXPECT_STREQ("Access is denied. ", s.AsCString());
+
+  s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("Negotiation timed out ", s.AsCString());
+
+  s.SetError(16000, ErrorType::eErrorTypeWin32);
+  EXPECT_STREQ("unknown error", s.AsCString());

I'm not sure, can we rely on the fact that messages are the same in different 
versions of Windows?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53092



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:801-804
+  if (m_filespec_ap.get())
+return m_filespec_ap->GetSize();
+
+  m_filespec_ap.reset(new FileSpecList());

I'm afraid of a race condition here. It seems we can occur here in different 
threads simultaneously (due to the mutex lock at the line 810), but it is not 
thread-safe to change and read //same// unique_ptr in different threads 
simultaneously. It is safe to //move// it between threads, but it's not the 
case. I would guard all actions on unique_ptr with the mutex.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:881-885
+  auto num_modules = ParseDependentModules();
+  auto original_size = files.GetSize();
+
+  for (unsigned i = 0; i < num_modules; ++i)
+files.AppendIfUnique(m_filespec_ap->GetFileSpecAtIndex(i));

Also consider the following scenario:

`Thread A`: `GetDependentModules` -> `ParseDependentModules` -> e.g. line 837, 
`idx` is greater than `0`, but less than `num_entries`;
`Thread B`: `GetDependentModules` (or `DumpDependentModules`) -> 
`ParseDependentModules` -> `ParseDependentModules` returns `num_modules` less 
than actual;
`Thread A`: continues to write into `m_filespec_ap`;
`Thread B`: reads `m_filespec_ap` which is modified at the time by `Thread A`.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1100-1104
+  auto num_modules = ParseDependentModules();
+  if (num_modules > 0) {
+s->PutCString("Dependent Modules\n");
+for (unsigned i = 0; i < num_modules; ++i) {
+  auto spec = m_filespec_ap->GetFileSpecAtIndex(i);

The same situation as I've described above.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53094



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53096: [lldb-test] Add a lit test for dependent modules in PECOFF

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov accepted this revision.
aleksandr.urakov added a comment.
This revision is now accepted and ready to land.

LGTM after https://reviews.llvm.org/D53094 will be done!


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53096



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Down to just modifying the DWARFFormValue constructor to be able to take a CU 
only. Looks good.




Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:241-242
+for (uint32_t i = 0; i < numAttributes; ++i) {
+  DWARFFormValue form_value;
+  form_value.SetCompileUnit(cu);
+

Make either a constructor that takes just a CU or provide a default value for 
the dw_form_t form? Then this code will be:

```
DWARFFormValue form_value(cu);
```



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:429-430
+for (uint32_t i = 0; i < numAttributes; ++i) {
+  DWARFFormValue form_value;
+  form_value.SetCompileUnit(cu);
+

Make either a constructor that takes just a CU or provide a default value for 
the dw_form_t form? Then this code will be:

```
DWARFFormValue form_value(cu);
```



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:625-626
+for (uint32_t i = 0; i < numAttributes; ++i) {
+  DWARFFormValue form_value;
+  form_value.SetCompileUnit(cu);
+  dw_attr_t attr;

Make either a constructor that takes just a CU or provide a default value for 
the dw_form_t form? Then this code will be:

```
DWARFFormValue form_value(cu);
```



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:803-804
+for (uint32_t i = 0; i < num_attributes; ++i) {
+  DWARFFormValue form_value;
+  form_value.SetCompileUnit(cu);
+

Make either a constructor that takes just a CU or provide a default value for 
the dw_form_t form? Then this code will be:

```
DWARFFormValue form_value(cu);
```


https://reviews.llvm.org/D52689



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344250 - [lldb] rename MinOS::minor to MinOS::minor_version etc. NFC

[Eric Liu via lldb-commits]

Author: ioeric
Date: Thu Oct 11 07:44:12 2018
New Revision: 344250

URL: http://llvm.org/viewvc/llvm-project?rev=344250&view=rev
Log:
[lldb] rename MinOS::minor to MinOS::minor_version etc. NFC

The constructor initializer minor(...)/major(...) can be confused with system
macros `#define minor(...)` on some platforms.

Modified:
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=344250&r1=344249&r2=344250&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Thu Oct 11 
07:44:12 2018
@@ -4944,11 +4944,11 @@ namespace {
   };
 
   struct MinOS {
-uint32_t major, minor, patch;
+uint32_t major_version, minor_version, patch_version;
 MinOS(uint32_t version)
-: major(version >> 16),
-  minor((version >> 8) & 0xffu),
-  patch(version & 0xffu) {}
+: major_version(version >> 16),
+  minor_version((version >> 8) & 0xffu),
+  patch_version(version & 0xffu) {}
   };
 } // namespace
 
@@ -5006,8 +5006,8 @@ bool ObjectFileMachO::GetArchitecture(co
 data.GetByteOrder(), &version_min) == 0)
 break;
   MinOS min_os(version_min.version);
-  os << GetOSName(load_cmd.cmd) << min_os.major << '.' << min_os.minor
- << '.' << min_os.patch;
+  os << GetOSName(load_cmd.cmd) << min_os.major_version << '.'
+ << min_os.minor_version << '.' << min_os.patch_version;
   triple.setOSName(os.str());
   return true;
 }
@@ -5037,8 +5037,8 @@ bool ObjectFileMachO::GetArchitecture(co
   OSEnv os_env(build_version.platform);
   if (os_env.os_type.empty())
 continue;
-  os << os_env.os_type << min_os.major << '.' << min_os.minor << '.'
- << min_os.patch;
+  os << os_env.os_type << min_os.major_version << '.'
+ << min_os.minor_version << '.' << min_os.patch_version;
   triple.setOSName(os.str());
   if (!os_env.environment.empty())
 triple.setEnvironmentName(os_env.environment);


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

[George Rimar via Phabricator via lldb-commits]

grimar updated this revision to Diff 169209.
grimar added a comment.

- Addressed review comments.


https://reviews.llvm.org/D52689

Files:
  lit/Breakpoint/Inputs/implicit_const_form_support.yaml
  lit/Breakpoint/implicit_const_form_support.test
  source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h

Index: source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -56,12 +56,17 @@
   };
 
   DWARFFormValue();
+  DWARFFormValue(const DWARFUnit *cu);
   DWARFFormValue(const DWARFUnit *cu, dw_form_t form);
   const DWARFUnit *GetCompileUnit() const { return m_cu; }
   void SetCompileUnit(const DWARFUnit *cu) { m_cu = cu; }
   dw_form_t Form() const { return m_form; }
+  dw_form_t& FormRef() { return m_form; }
   void SetForm(dw_form_t form) { m_form = form; }
   const ValueType &Value() const { return m_value; }
+  ValueType &ValueRef() { return m_value; }
+  void SetValue(const ValueType &val) { m_value = val; }
+
   void Dump(lldb_private::Stream &s) const;
   bool ExtractValue(const lldb_private::DWARFDataExtractor &data,
 lldb::offset_t *offset_ptr);
Index: source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -154,6 +154,8 @@
 
 DWARFFormValue::DWARFFormValue() : m_cu(NULL), m_form(0), m_value() {}
 
+DWARFFormValue::DWARFFormValue(const DWARFUnit *cu) : m_cu(cu), m_form(0), m_value() {}
+
 DWARFFormValue::DWARFFormValue(const DWARFUnit *cu, dw_form_t form)
 : m_cu(cu), m_form(form), m_value() {}
 
@@ -165,6 +167,9 @@
 
 bool DWARFFormValue::ExtractValue(const DWARFDataExtractor &data,
   lldb::offset_t *offset_ptr) {
+  if (m_form == DW_FORM_implicit_const)
+return true;
+
   bool indirect = false;
   bool is_block = false;
   m_value.data = NULL;
@@ -366,6 +371,7 @@
 
   // 0 bytes values (implied from DW_FORM)
   case DW_FORM_flag_present:
+  case DW_FORM_implicit_const:
 return true;
 
 // 1 byte values
@@ -822,6 +828,7 @@
 case DW_FORM_ref_sig8:
 case DW_FORM_GNU_str_index:
 case DW_FORM_GNU_addr_index:
+case DW_FORM_implicit_const:
   return true;
 default:
   break;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -178,7 +178,7 @@
   DumpAttribute(SymbolFileDWARF *dwarf2Data, const DWARFUnit *cu,
 const lldb_private::DWARFDataExtractor &debug_info_data,
 lldb::offset_t *offset_ptr, lldb_private::Stream &s,
-dw_attr_t attr, dw_form_t form);
+dw_attr_t attr, DWARFFormValue &form_value);
   // This one dumps the comp unit name, objfile name and die offset for this die
   // so the stream S.
   void DumpLocation(SymbolFileDWARF *dwarf2Data, DWARFUnit *cu,
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -174,6 +174,10 @@
   debug_info_data.GetU32(&offset);
 break;
 
+  case DW_FORM_implicit_const:
+form_size = 0;
+break;
+
   default:
 *offset_ptr = m_offset;
 return false;
@@ -233,15 +237,14 @@
 
 // Skip all data in the .debug_info for the attributes
 const uint32_t numAttributes = abbrevDecl->NumAttributes();
-uint32_t i;
-dw_attr_t attr;
-dw_form_t form;
-for (i = 0; i < numAttributes; ++i) {
-  abbrevDecl->GetAttrAndFormByIndexUnchecked(i, attr, form);
+for (uint32_t i = 0; i < numAttributes; ++i) {
+  DWARFFormValue form_value(cu);
+  dw_attr_t attr;
+  abbrevDecl->GetAttrAndFormValueByIndex(i, attr, form_value);
+  dw_form_t form = form_value.Form();
 
   if (isCompileUnitTag &&
   ((attr == DW_AT_entry_pc) || (attr == DW_AT_low_pc))) {
-DWARFFormValue form_value(cu, form);
 if (form_value.ExtractValue(debug_info_data, &offset)) {
   if (attr == DW_AT_low_pc || attr == DW_A

[Lldb-commits] [lldb] r344252 - [lldb] Surpress copy-elison warning.

[Eric Liu via lldb-commits]

Author: ioeric
Date: Thu Oct 11 07:52:33 2018
New Revision: 344252

URL: http://llvm.org/viewvc/llvm-project?rev=344252&view=rev
Log:
[lldb] Surpress copy-elison warning.

Modified:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp?rev=344252&r1=344251&r2=344252&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp Thu 
Oct 11 07:52:33 2018
@@ -75,7 +75,7 @@ static std::unique_ptr loadPDBF
   if (auto EC = File->parseStreamData())
 return nullptr;
 
-  return std::move(File);
+  return File;
 }
 
 static std::unique_ptr
@@ -119,7 +119,7 @@ loadMatchingPDBFile(std::string exe_path
 
   if (expected_info->getGuid() != guid)
 return nullptr;
-  return std::move(pdb);
+  return pdb;
 }
 
 static bool IsFunctionPrologue(const CompilandIndexItem &cci,


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)

[George Rimar via Phabricator via lldb-commits]

grimar created this revision.
grimar added reviewers: LLDB, clayborg.
Herald added a subscriber: JDevlieghere.

The patch implements the support for DW_RLE_base_address and DW_RLE_offset_pair
.debug_rnglists  entries


https://reviews.llvm.org/D53140

Files:
  lit/Breakpoint/Inputs/debug_rnglist_offset_pair.yaml
  lit/Breakpoint/debug_rnglist_offset_pair.test
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -53,8 +53,7 @@
 class DWARFDebugInfo;
 class DWARFDebugInfoEntry;
 class DWARFDebugLine;
-class DWARFDebugRanges;
-class DWARFDebugRngLists;
+class DWARFDebugRangesBase;
 class DWARFDeclContext;
 class DWARFDIECollection;
 class DWARFFormValue;
@@ -263,9 +262,9 @@
 
   const DWARFDebugInfo *DebugInfo() const;
 
-  DWARFDebugRanges *DebugRanges();
+  DWARFDebugRangesBase *DebugRanges();
 
-  const DWARFDebugRanges *DebugRanges() const;
+  const DWARFDebugRangesBase *DebugRanges() const;
 
   static bool SupportedVersion(uint16_t version);
 
@@ -506,7 +505,7 @@
   typedef std::shared_ptr> DIERefSetSP;
   typedef std::unordered_map NameToOffsetMap;
   NameToOffsetMap m_function_scope_qualified_name_map;
-  std::unique_ptr m_ranges;
+  std::unique_ptr m_ranges;
   UniqueDWARFASTTypeMap m_unique_ast_type_map;
   DIEToTypePtr m_die_to_type;
   DIEToVariableSP m_die_to_variable_sp;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -753,7 +753,7 @@
   return NULL;
 }
 
-DWARFDebugRanges *SymbolFileDWARF::DebugRanges() {
+DWARFDebugRangesBase *SymbolFileDWARF::DebugRanges() {
   if (m_ranges.get() == NULL) {
 static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
 Timer scoped_timer(func_cat, "%s this = %p", LLVM_PRETTY_FUNCTION,
@@ -770,7 +770,7 @@
   return m_ranges.get();
 }
 
-const DWARFDebugRanges *SymbolFileDWARF::DebugRanges() const {
+const DWARFDebugRangesBase *SymbolFileDWARF::DebugRanges() const {
   return m_ranges.get();
 }
 
@@ -3328,15 +3328,10 @@
   case DW_AT_start_scope: {
 if (form_value.Form() == DW_FORM_sec_offset) {
   DWARFRangeList dwarf_scope_ranges;
-  const DWARFDebugRanges *debug_ranges = DebugRanges();
-  debug_ranges->FindRanges(die.GetCU()->GetRangesBase(),
+  const DWARFDebugRangesBase *debug_ranges = DebugRanges();
+  debug_ranges->FindRanges(die.GetCU(),
form_value.Unsigned(),
dwarf_scope_ranges);
-
-  // All DW_AT_start_scope are relative to the base address of the
-  // compile unit. We add the compile unit base address to make
-  // sure all the addresses are properly fixed up.
-  dwarf_scope_ranges.Slide(die.GetCU()->GetBaseAddress());
 } else {
   // TODO: Handle the case when DW_AT_start_scope have form
   // constant. The
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
@@ -15,18 +15,26 @@
 
 #include 
 
-class DWARFDebugRanges {
+class DWARFDebugRangesBase {
+public:
+  virtual ~DWARFDebugRangesBase(){};
+
+  virtual void Extract(SymbolFileDWARF *dwarf2Data) = 0;
+  virtual bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
+  DWARFRangeList &range_list) const = 0;
+};
+
+class DWARFDebugRanges final : public DWARFDebugRangesBase {
 public:
   DWARFDebugRanges();
-  virtual ~DWARFDebugRanges();
-  virtual void Extract(SymbolFileDWARF *dwarf2Data);
+
+  virtual void Extract(SymbolFileDWARF *dwarf2Data) override;
+  virtual bool FindRanges(const DWARFUnit *cu, dw_offset_t debug_ranges_offset,
+  DWARFRangeList &range_list) const override;
 
   static void Dump(lldb_private::Stream &s,
const lldb_private::DWARFDataExtractor &debug_ranges_data,
lldb::offset_t *offset_ptr, dw_addr_t cu_base_addr);
-  bool FindRanges(dw_addr_t debug_ranges_base,
-  dw_offset_t debug_ranges_offset,
-  DWARFRangeList &range_list) const;
 
 protected:
   bool Extract(SymbolFileDWARF *dwarf2Data, lldb::offset_t *offset_ptr,
@@ -39,16 +47,25 @@
 };
 
 // DWARF v5 .debug_rnglists section.
-cl

[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Thanks for making the changes. Looks good!


https://reviews.llvm.org/D52689



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

See inlined comments.




Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp:150-151
+case DW_RLE_base_address: {
+  dw_addr_t base = data.GetMaxU64(offset_ptr, addrSize);
+  rangeList.push_back({DW_RLE_base_address, base, 0});
+  break;

Might be nice to not push a base address entry and store the base address 
locally and fixup any DW_RLE_offset_pair entries on the fly?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h:51-55
+  struct RngListEntry {
+uint8_t encoding;
+uint64_t value0;
+uint64_t value1;
+  };

Do we really need to store all this? Can't we just convert to address ranges on 
the fly in DWARFDebugRngLists::Extract? With the current DW_RLE_base_address 
and DW_RLE_offset_pair stuff we can store the base address locally inside the 
DWARFDebugRngLists::Extract function and skip pushing an entry for it and then 
convert any DW_RLE_offset_pair stuff into addresses by adding the base address 
before pushing the range. Or will this be required to support other opcodes?


https://reviews.llvm.org/D53140



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r344154 - Create a SymbolFile plugin for cross-platform PDB access.

[Davide Italiano via lldb-commits]

On Wed, Oct 10, 2018 at 9:40 AM Zachary Turner via lldb-commits
 wrote:
>
> Author: zturner
> Date: Wed Oct 10 09:39:07 2018
> New Revision: 344154
>
> URL: http://llvm.org/viewvc/llvm-project?rev=344154&view=rev
> Log:
> Create a SymbolFile plugin for cross-platform PDB access.
>

Hey Zach, I'm afraid this broke the MacOS Cmake bots (see, e.g.
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/11020/)

May I ask you to take a look?

The failure pattern looks like:


FAIL: lldb :: SymbolFile/NativePDB/disassembly.cpp (48 of 1357)

clang-8: warning:
'/Users/davide/work/llvm-project-20170507/lldb/lit/SymbolFile/NativePDB/disassembly.cpp'
treated as the '/U' option [-Wslash-u-filename]

clang-8: note: Use '--' to treat subsequent arguments as filenames

clang-8: error: no input files


So it's hopefully something relatively straightforward to fix. Please
let me know if you need other informations.

Thanks,

--
Davide
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r344154 - Create a SymbolFile plugin for cross-platform PDB access.

[Zachary Turner via lldb-commits]

I'm working on a fix for this and several other issues right now.  For this
one I think we just need to add -- on the command line.

On Thu, Oct 11, 2018 at 9:36 AM Davide Italiano 
wrote:

> On Wed, Oct 10, 2018 at 9:40 AM Zachary Turner via lldb-commits
>  wrote:
> >
> > Author: zturner
> > Date: Wed Oct 10 09:39:07 2018
> > New Revision: 344154
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=344154&view=rev
> > Log:
> > Create a SymbolFile plugin for cross-platform PDB access.
> >
>
> Hey Zach, I'm afraid this broke the MacOS Cmake bots (see, e.g.
> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/11020/)
>
> May I ask you to take a look?
>
> The failure pattern looks like:
>
> 
> FAIL: lldb :: SymbolFile/NativePDB/disassembly.cpp (48 of 1357)
>
> clang-8: warning:
>
> '/Users/davide/work/llvm-project-20170507/lldb/lit/SymbolFile/NativePDB/disassembly.cpp'
> treated as the '/U' option [-Wslash-u-filename]
>
> clang-8: note: Use '--' to treat subsequent arguments as filenames
>
> clang-8: error: no input files
> 
>
> So it's hopefully something relatively straightforward to fix. Please
> let me know if you need other informations.
>
> Thanks,
>
> --
> Davide
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r344154 - Create a SymbolFile plugin for cross-platform PDB access.

[Davide Italiano via lldb-commits]

Great, thanks for the update.
I'll wait then. There's also an assertion failure in DebugInfo/MSF,
which was, incidentally, caused by another test we added yesterday. I
think it might be related (be in your area).

Thanks,

--
Davide
On Thu, Oct 11, 2018 at 9:47 AM Zachary Turner  wrote:
>
> I'm working on a fix for this and several other issues right now.  For this 
> one I think we just need to add -- on the command line.
>
> On Thu, Oct 11, 2018 at 9:36 AM Davide Italiano  wrote:
>>
>> On Wed, Oct 10, 2018 at 9:40 AM Zachary Turner via lldb-commits
>>  wrote:
>> >
>> > Author: zturner
>> > Date: Wed Oct 10 09:39:07 2018
>> > New Revision: 344154
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=344154&view=rev
>> > Log:
>> > Create a SymbolFile plugin for cross-platform PDB access.
>> >
>>
>> Hey Zach, I'm afraid this broke the MacOS Cmake bots (see, e.g.
>> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/11020/)
>>
>> May I ask you to take a look?
>>
>> The failure pattern looks like:
>>
>> 
>> FAIL: lldb :: SymbolFile/NativePDB/disassembly.cpp (48 of 1357)
>>
>> clang-8: warning:
>> '/Users/davide/work/llvm-project-20170507/lldb/lit/SymbolFile/NativePDB/disassembly.cpp'
>> treated as the '/U' option [-Wslash-u-filename]
>>
>> clang-8: note: Use '--' to treat subsequent arguments as filenames
>>
>> clang-8: error: no input files
>> 
>>
>> So it's hopefully something relatively straightforward to fix. Please
>> let me know if you need other informations.
>>
>> Thanks,
>>
>> --
>> Davide
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344269 - Better support for POSIX paths in PDBs.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Thu Oct 11 11:01:55 2018
New Revision: 344269

URL: http://llvm.org/viewvc/llvm-project?rev=344269&view=rev
Log:
Better support for POSIX paths in PDBs.

While it doesn't make a *ton* of sense for POSIX paths to be
in PDBs, it's possible to occur in real scenarios involving
cross compilation.

The tools need to be able to handle this, because certain types
of debugging scenarios are possible without a running process
and so don't necessarily require you to be on a Windows system.
These include post-mortem debugging and binary forensics (e.g.
using a debugger to disassemble functions and examine symbols
without running the process).

There's changes in clang, LLD, and lldb in this patch.  After
this the cross-platform disassembly and source-list tests pass
on Linux.

Furthermore, the behavior of LLD can now be summarized by a much
simpler rule than before: Unless you specify /pdbsourcepath and
/pdbaltpath, the PDB ends up with paths that are valid within
the context of the machine that the link is performed on.

Differential Revision: https://reviews.llvm.org/D53149

Modified:
lldb/trunk/lit/SymbolFile/NativePDB/source-list.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Modified: lldb/trunk/lit/SymbolFile/NativePDB/source-list.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/NativePDB/source-list.cpp?rev=344269&r1=344268&r2=344269&view=diff
==
--- lldb/trunk/lit/SymbolFile/NativePDB/source-list.cpp (original)
+++ lldb/trunk/lit/SymbolFile/NativePDB/source-list.cpp Thu Oct 11 11:01:55 2018
@@ -25,7 +25,7 @@ int main(int argc, char **argv) {
 // changing this file.
 
 // CHECK: (lldb) source list -n main
-// CHECK: File: D:\src\llvm-mono\lldb\lit\SymbolFile\NativePDB\source-list.cpp
+// CHECK: File: {{.*}}source-list.cpp
 // CHECK:10
 // CHECK:11// Some context lines before
 // CHECK:12   // the function.

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp?rev=344269&r1=344268&r2=344269&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp Thu Oct 
11 11:01:55 2018
@@ -42,11 +42,8 @@ static bool IsMainFile(llvm::StringRef m
   if (llvm::sys::fs::equivalent(main, other))
 return true;
 
-  // FIXME: If we ever want to support PDB debug info for non-Windows systems
-  // the following check will be wrong, but we need a way to store the host
-  // information in the PDB.
   llvm::SmallString<64> normalized(other);
-  llvm::sys::path::native(normalized, llvm::sys::path::Style::windows);
+  llvm::sys::path::native(normalized);
   return main.equals_lower(normalized);
 }
 
@@ -156,7 +153,8 @@ CompileUnitIndex::GetOrCreateCompiland(P
   // name until we find it, and we can cache that one since the memory is 
backed
   // by a contiguous chunk inside the mapped PDB.
   llvm::SmallString<64> main_file = GetMainSourceFile(*cci);
-  llvm::sys::path::native(main_file, llvm::sys::path::Style::windows);
+  std::string s = main_file.str();
+  llvm::sys::path::native(main_file);
 
   uint32_t file_count = modules.getSourceFileCount(modi);
   cci->m_file_list.reserve(file_count);

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp?rev=344269&r1=344268&r2=344269&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp Thu 
Oct 11 11:01:55 2018
@@ -530,7 +530,9 @@ bool SymbolFileNativePDB::ParseCompileUn
   lldbassert(cci);
 
   for (llvm::StringRef f : cci->m_file_list) {
-FileSpec spec(f, false, FileSpec::Style::windows);
+FileSpec::Style style =
+f.startswith("/") ? FileSpec::Style::posix : FileSpec::Style::windows;
+FileSpec spec(f, false, style);
 support_files.Append(spec);
   }
 


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 169259.
shafik marked an inline comment as done.
shafik added a comment.

Removing uneeded code and making test NO_DEBUG_INFO_TESTCASE


https://reviews.llvm.org/D52851

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ThreadPlanStepThrough.cpp

Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -13,6 +13,7 @@
 // Project includes
 #include "lldb/Target/ThreadPlanStepThrough.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Target/CPPLanguageRuntime.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Process.h"
@@ -95,6 +96,15 @@
 if (objc_runtime)
   m_sub_plan_sp =
   objc_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
+
+CPPLanguageRuntime *cpp_runtime =
+m_thread.GetProcess()->GetCPPLanguageRuntime();
+
+// If the ObjC runtime did not provide us with a step though plan then if we
+// have it check the C++ runtime for a step though plan.
+if (!m_sub_plan_sp.get() && cpp_runtime)
+  m_sub_plan_sp =
+  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);
   }
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInRange.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -158,7 +159,6 @@
   // We do this by find the first < and , and extracting in between.
   //
   // This covers the case of the lambda known at compile time.
-  //
   size_t first_open_angle_bracket = vtable_name.find('<') + 1;
   size_t first_comma = vtable_name.find_first_of(',');
 
@@ -262,3 +262,76 @@
 
   return optional_info;
 }
+
+lldb::ThreadPlanSP
+CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
+ bool stop_others) {
+  ThreadPlanSP ret_plan_sp;
+
+  lldb::addr_t curr_pc = thread.GetRegisterContext()->GetPC();
+
+  TargetSP target_sp(thread.CalculateTarget());
+
+  if (target_sp->GetSectionLoadList().IsEmpty())
+return ret_plan_sp;
+
+  Address pc_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target_sp->GetSectionLoadList().ResolveLoadAddress(curr_pc,
+  pc_addr_resolved))
+return ret_plan_sp;
+
+  target_sp->GetImages().ResolveSymbolContextForAddress(
+  pc_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == nullptr)
+return ret_plan_sp;
+
+  llvm::StringRef function_name(symbol->GetName().GetCString());
+
+  // Handling the case where we are attempting to step into std::function.
+  // The behavior will be that we will attempt to obtain the wrapped
+  // callable via FindLibCppStdFunctionCallableInfo() and if we find it we
+  // will return a ThreadPlanRunToAddress to the callable. Therefore we will
+  // step into the wrapped callable.
+  //
+  bool found_expected_start_string =
+  function_name.startswith("std::__1::function<");
+
+  if (!found_expected_start_string)
+return ret_plan_sp;
+
+  AddressRange range_of_curr_func;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range_of_curr_func);
+
+  StackFrameSP frame = thread.GetStackFrameAtIndex(0);
+
+  if (frame) {
+ValueObjectSP value_sp = frame->FindVariable(ConstString("this"));
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo callable_info =
+FindLibCppStdFunctionCallableInfo(value_sp);
+
+if (callable_info.callable_case != LibCppStdFunctionCallableCase::Invalid &&
+value_sp->GetValueIsValid()) {
+  // We found the std::function wrapped callable and we have its address.
+  // We now create a ThreadPlan to run to the callable.
+  ret_plan_sp.reset(new ThreadPlanRunToAddress(
+  thread, callable_info.callable_address, stop_others));
+  return ret_plan_sp;
+} else {
+  // We are in std::function but we could not obtain the callable.
+  // We create a ThreadPlan to keep stepping through using the address range
+  // of the current function.
+  ret_plan_sp.reset(new ThreadPlanStepInRange(thread, range_of_curr_func,
+  

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@jingham removed code and made it NO_DEBUG_INFO_TESTCASE


https://reviews.llvm.org/D52851



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

This looks great, thanks for making this work, it will be SO helpful for 
debugging std::function uses.


https://reviews.llvm.org/D52851



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344275 - Don't mark an LC_BUILD_VERSION as giving us a

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Thu Oct 11 11:37:53 2018
New Revision: 344275

URL: http://llvm.org/viewvc/llvm-project?rev=344275&view=rev
Log:
Don't mark an LC_BUILD_VERSION as giving us a 
correct version if it has a major verison 0.

Modified:
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=344275&r1=344274&r2=344275&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Thu Oct 11 
11:37:53 2018
@@ -5914,8 +5914,8 @@ uint32_t ObjectFileMachO::GetSDKVersion(
 m_sdk_versions.push_back ();
 m_sdk_versions.push_back (yy);
 m_sdk_versions.push_back (zz);
+success = true;
 }
-success = true;
 }
 offset = load_cmd_offset + lc.cmdsize;
 }


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344277 - Fix this comment so it is consistent with all the others.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Thu Oct 11 11:41:34 2018
New Revision: 344277

URL: http://llvm.org/viewvc/llvm-project?rev=344277&view=rev
Log:

Fix this comment so it is consistent with all the others.

Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp

Modified: 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp?rev=344277&r1=344276&r2=344277&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp Thu 
Oct 11 11:41:34 2018
@@ -108,7 +108,7 @@ PlatformSP PlatformRemoteAppleBridge::Cr
   }
   if (create) {
 switch (triple.getOS()) {
-// FIXMEJSM case llvm::Triple::BridgeOS: 
+// NEED_BRIDGEOS_TRIPLE case llvm::Triple::BridgeOS: 
   break;
 
 default:


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM too. Thanks for taking the time to address the comments.


https://reviews.llvm.org/D52851



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344279 - Revert SymbolFileNativePDB plugin.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Thu Oct 11 11:45:44 2018
New Revision: 344279

URL: http://llvm.org/viewvc/llvm-project?rev=344279&view=rev
Log:
Revert SymbolFileNativePDB plugin.

This was originally causing some test failures on non-Windows
platforms, which required fixes in the compiler and linker.  After
those fixes, however, other tests started failing.  Reverting
temporarily until I can address everything.

Removed:
lldb/trunk/lit/SymbolFile/NativePDB/
lldb/trunk/source/Plugins/SymbolFile/NativePDB/
Modified:
lldb/trunk/include/lldb/Utility/LLDBAssert.h
lldb/trunk/lit/lit.cfg
lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Modified: lldb/trunk/include/lldb/Utility/LLDBAssert.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/LLDBAssert.h?rev=344279&r1=344278&r2=344279&view=diff
==
--- lldb/trunk/include/lldb/Utility/LLDBAssert.h (original)
+++ lldb/trunk/include/lldb/Utility/LLDBAssert.h Thu Oct 11 11:45:44 2018
@@ -14,8 +14,7 @@
 #define lldbassert(x) assert(x)
 #else
 #define lldbassert(x)  
\
-  lldb_private::lldb_assert(static_cast(x), #x, __FUNCTION__, __FILE__,  
\
-__LINE__)
+  lldb_private::lldb_assert(x, #x, __FUNCTION__, __FILE__, __LINE__)
 #endif
 
 namespace lldb_private {

Modified: lldb/trunk/lit/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.cfg?rev=344279&r1=344278&r2=344279&view=diff
==
--- lldb/trunk/lit/lit.cfg (original)
+++ lldb/trunk/lit/lit.cfg Thu Oct 11 11:45:44 2018
@@ -64,8 +64,6 @@ lldb = "%s -S %s/lit-lldb-init" % (lit.u
config.test_source_root)
 
 lldbmi = lit.util.which('lldb-mi', lldb_tools_dir)
-if lldbmi:
-config.available_features.add('lldb-mi')
 
 if not os.path.exists(config.cc):
 config.cc = lit.util.which(config.cc, config.environment['PATH'])
@@ -92,8 +90,7 @@ if platform.system() in ['OpenBSD']:
 config.substitutions.append(('%cc', config.cc))
 config.substitutions.append(('%cxx', config.cxx))
 
-if lldbmi:
-  config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
+config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
 config.substitutions.append(('%lldb', lldb))
 
 if debugserver is not None:

Modified: lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt?rev=344279&r1=344278&r2=344279&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt Thu Oct 11 11:45:44 2018
@@ -1,4 +1,3 @@
 add_subdirectory(DWARF)
 add_subdirectory(Symtab)
-add_subdirectory(NativePDB)
 add_subdirectory(PDB)

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt?rev=344279&r1=344278&r2=344279&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt Thu Oct 11 11:45:44 
2018
@@ -9,7 +9,6 @@ add_lldb_library(lldbPluginSymbolFilePDB
 lldbCore
 lldbSymbol
lldbUtility
-  lldbPluginSymbolFileNativePDB
   LINK_COMPONENTS
 DebugInfoPDB
 Support

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=344279&r1=344278&r2=344279&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Thu Oct 11 
11:45:44 2018
@@ -46,7 +46,6 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" // For 
IsCPPMangledName
-#include "Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h"
 #include "Plugins/SymbolFile/PDB/PDBASTParser.h"
 #include "Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h"
 
@@ -75,31 +74,14 @@ bool ShouldAddLine(uint32_t requested_li
 }
 } // namespace
 
-static bool ShouldUseNativeReader() {
-#if !defined(_WIN32)
-  return true;
-#endif
-  llvm::StringRef use_native = ::getenv("LLDB_USE_NATIVE_PDB_READER");
-  return use_native.equals_lower("on") || use_native.equals_lower("yes") ||
- use_native.equals_lower("1") || use_native.equals_lower("true");
-}
-
 void SymbolFilePDB::Initialize() {
-  if (ShouldUseNativeReader()) {
-  

Re: [Lldb-commits] [lldb] r344279 - Revert SymbolFileNativePDB plugin.

[Davide Italiano via lldb-commits]

thanks.
On Thu, Oct 11, 2018 at 11:47 AM Zachary Turner via lldb-commits
 wrote:
>
> Author: zturner
> Date: Thu Oct 11 11:45:44 2018
> New Revision: 344279
>
> URL: http://llvm.org/viewvc/llvm-project?rev=344279&view=rev
> Log:
> Revert SymbolFileNativePDB plugin.
>
> This was originally causing some test failures on non-Windows
> platforms, which required fixes in the compiler and linker.  After
> those fixes, however, other tests started failing.  Reverting
> temporarily until I can address everything.
>
> Removed:
> lldb/trunk/lit/SymbolFile/NativePDB/
> lldb/trunk/source/Plugins/SymbolFile/NativePDB/
> Modified:
> lldb/trunk/include/lldb/Utility/LLDBAssert.h
> lldb/trunk/lit/lit.cfg
> lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt
> lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt
> lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
>
> Modified: lldb/trunk/include/lldb/Utility/LLDBAssert.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/LLDBAssert.h?rev=344279&r1=344278&r2=344279&view=diff
> ==
> --- lldb/trunk/include/lldb/Utility/LLDBAssert.h (original)
> +++ lldb/trunk/include/lldb/Utility/LLDBAssert.h Thu Oct 11 11:45:44 2018
> @@ -14,8 +14,7 @@
>  #define lldbassert(x) assert(x)
>  #else
>  #define lldbassert(x)
>   \
> -  lldb_private::lldb_assert(static_cast(x), #x, __FUNCTION__, 
> __FILE__,  \
> -__LINE__)
> +  lldb_private::lldb_assert(x, #x, __FUNCTION__, __FILE__, __LINE__)
>  #endif
>
>  namespace lldb_private {
>
> Modified: lldb/trunk/lit/lit.cfg
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.cfg?rev=344279&r1=344278&r2=344279&view=diff
> ==
> --- lldb/trunk/lit/lit.cfg (original)
> +++ lldb/trunk/lit/lit.cfg Thu Oct 11 11:45:44 2018
> @@ -64,8 +64,6 @@ lldb = "%s -S %s/lit-lldb-init" % (lit.u
> config.test_source_root)
>
>  lldbmi = lit.util.which('lldb-mi', lldb_tools_dir)
> -if lldbmi:
> -config.available_features.add('lldb-mi')
>
>  if not os.path.exists(config.cc):
>  config.cc = lit.util.which(config.cc, config.environment['PATH'])
> @@ -92,8 +90,7 @@ if platform.system() in ['OpenBSD']:
>  config.substitutions.append(('%cc', config.cc))
>  config.substitutions.append(('%cxx', config.cxx))
>
> -if lldbmi:
> -  config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
> +config.substitutions.append(('%lldbmi', lldbmi + " --synchronous"))
>  config.substitutions.append(('%lldb', lldb))
>
>  if debugserver is not None:
>
> Modified: lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt?rev=344279&r1=344278&r2=344279&view=diff
> ==
> --- lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt (original)
> +++ lldb/trunk/source/Plugins/SymbolFile/CMakeLists.txt Thu Oct 11 11:45:44 
> 2018
> @@ -1,4 +1,3 @@
>  add_subdirectory(DWARF)
>  add_subdirectory(Symtab)
> -add_subdirectory(NativePDB)
>  add_subdirectory(PDB)
>
> Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt?rev=344279&r1=344278&r2=344279&view=diff
> ==
> --- lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt (original)
> +++ lldb/trunk/source/Plugins/SymbolFile/PDB/CMakeLists.txt Thu Oct 11 
> 11:45:44 2018
> @@ -9,7 +9,6 @@ add_lldb_library(lldbPluginSymbolFilePDB
>  lldbCore
>  lldbSymbol
> lldbUtility
> -  lldbPluginSymbolFileNativePDB
>LINK_COMPONENTS
>  DebugInfoPDB
>  Support
>
> Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=344279&r1=344278&r2=344279&view=diff
> ==
> --- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
> +++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Thu Oct 11 
> 11:45:44 2018
> @@ -46,7 +46,6 @@
>  #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
>
>  #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" // For 
> IsCPPMangledName
> -#include "Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h"
>  #include "Plugins/SymbolFile/PDB/PDBASTParser.h"
>  #include "Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h"
>
> @@ -75,31 +74,14 @@ bool ShouldAddLine(uint32_t requested_li
>  }
>  } // namespace
>
> -static bool ShouldUseNativeReader() {
> -#if !defined(_WIN32)
> -  return true;
> -#endif

[Lldb-commits] [PATCH] D53166: [lldbsuite] Fix the filecheck functionality to work with Python 3

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova created this revision.
stella.stamenova added reviewers: zturner, asmith.
Herald added a subscriber: lldb-commits.

This is another string/byte conversion issue between Python 2 and 3. In Python 
2, the subprocess communication expects a byte string, but in Python 3, it 
expects bytes.

There are a pair of functions to_bytes/to_string in lit that implement the 
correct conversion, but they're not available throughout the lldb suite which 
is why this fix is 'in place' rather than calling one of these functions.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53166

Files:
  packages/Python/lldbsuite/test/lldbtest.py


Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -2227,6 +2227,10 @@
 # Get the error text if there was an error, and the regular text if 
not.
 output = self.res.GetOutput() if self.res.Succeeded() \
 else self.res.GetError()
+# In Python 2, communicate sends byte strings. In Python 3, 
communicate sends bytes.
+# If we got a string (and not a byte string), encode it before sending.
+if isinstance(output, str) and not isinstance(output, bytes):
+output = output.encode()
 
 # Assemble the absolute path to the check file. As a convenience for
 # LLDB inline tests, assume that the check file is a relative path to


Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -2227,6 +2227,10 @@
 # Get the error text if there was an error, and the regular text if not.
 output = self.res.GetOutput() if self.res.Succeeded() \
 else self.res.GetError()
+# In Python 2, communicate sends byte strings. In Python 3, communicate sends bytes.
+# If we got a string (and not a byte string), encode it before sending.
+if isinstance(output, str) and not isinstance(output, bytes):
+output = output.encode()
 
 # Assemble the absolute path to the check file. As a convenience for
 # LLDB inline tests, assume that the check file is a relative path to
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53166: [lldbsuite] Fix the filecheck functionality to work with Python 3

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: packages/Python/lldbsuite/test/lldbtest.py:2230-2233
+# In Python 2, communicate sends byte strings. In Python 3, 
communicate sends bytes.
+# If we got a string (and not a byte string), encode it before sending.
+if isinstance(output, str) and not isinstance(output, bytes):
+output = output.encode()

Ok I had to stare at this for a long time to figure out what was going on.  I 
think you need to update the comment here, because it makes it sounds like 
`output` is the result of a `Popen.communicate`.  But `output` is the result of 
a `debugger.HandleCommand()`.  

I think we should actually just leave this the way it was and fix it in the 
call to `Popen` (see below)



Comment at: packages/Python/lldbsuite/test/lldbtest.py:2247-2249
 subproc = Popen(filecheck_args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
 cmd_stdout, cmd_stderr = subproc.communicate(input=output)
 cmd_status = subproc.returncode

If I'm not mistaken, Python 3 can accept `stdin` as `bytes` or `string`, and 
either will work, it depends on the value of `universal_newlines`, and `stdout` 
will be in whatever format `stdin` was in.  If we pass 
`universal_newlines=True`, then it will expect a `string` and output a 
`string`, otherwise it expects a `bytes` and output a `bytes`.  Given that 
`FileCheck` is supposed to operate on textual data and not binary data, perhaps 
we should be doing that here?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53166



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

Unfortunately, the bots are broken because of the FileCheck issue, so I can't 
confirm with them, but I see a number of these tests fail in our local testing. 
Some fail on both Windows and Linux and some just fail on Linux. Here are the 
failing tests:

  Linux:
  lldb-Suite :: 
functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
  lldb-Suite :: 
functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
  lldb-Suite :: 
functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
  lldb-Suite :: 
functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
  lldb-Suite :: 
functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
  lldb-Suite :: 
functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
  lldb-Suite :: 
functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
  
  Windows:
  lldb-Suite :: 
functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
  lldb-Suite :: 
functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py

Let me know what you need to investigate.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

In https://reviews.llvm.org/D50478#1262710, @stella.stamenova wrote:

> Unfortunately, the bots are broken because of the FileCheck issue, so I can't 
> confirm with them, but I see a number of these tests fail in our local 
> testing. Some fail on both Windows and Linux and some just fail on Linux. 
> Here are the failing tests:
>
>   Linux:
>   lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
>  
>   Windows:
>   lldb-Suite :: 
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
>   lldb-Suite :: 
> functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
>
>
> Let me know what you need to investigate.


Strange, I didn't get any bot failure notifications in the days after this 
landed. Could you share the output from the failing tests?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Zachary Turner via lldb-commits]

See the other email thread.  The bots have been broken since September, all
of them complain about missing FileCheck executable

On Thu, Oct 11, 2018 at 2:58 PM Vedant Kumar via Phabricator <
revi...@reviews.llvm.org> wrote:

> vsk added a comment.
>
> In https://reviews.llvm.org/D50478#1262710, @stella.stamenova wrote:
>
> > Unfortunately, the bots are broken because of the FileCheck issue, so I
> can't confirm with them, but I see a number of these tests fail in our
> local testing. Some fail on both Windows and Linux and some just fail on
> Linux. Here are the failing tests:
> >
> >   Linux:
> >   lldb-Suite ::
> functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
> >
> >   Windows:
> >   lldb-Suite ::
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> >   lldb-Suite ::
> functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
> >
> >
> > Let me know what you need to investigate.
>
>
> Strange, I didn't get any bot failure notifications in the days after this
> landed. Could you share the output from the failing tests?
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D50478
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Zachary Turner via Phabricator via lldb-commits]

zturner added a subscriber: vsk.
zturner added a comment.

See the other email thread.  The bots have been broken since September, all
of them complain about missing FileCheck executable


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Zachary Turner via Phabricator via lldb-commits]

zturner added a subscriber: aprantl.
zturner added a comment.

https://reviews.llvm.org/D53002

Is the thread I'm referring to.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Zachary Turner via lldb-commits]

https://reviews.llvm.org/D53002

Is the thread I'm referring to.

On Thu, Oct 11, 2018 at 2:59 PM Zachary Turner via Phabricator <
revi...@reviews.llvm.org> wrote:

> zturner added a subscriber: vsk.
> zturner added a comment.
>
> See the other email thread.  The bots have been broken since September, all
> of them complain about missing FileCheck executable
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D50478
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: stella.stamenova, zturner.

This allows bots which haven't updated to pass in --filecheck to dotest.py to 
run more tests. FileCheck-dependent tests will continue to fail.


https://reviews.llvm.org/D53175

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -311,8 +311,8 @@
 # The CMake build passes in a path to a working FileCheck binary.
 configuration.filecheck = os.path.abspath(args.filecheck)
 else:
-logging.error('No valid FileCheck executable; aborting...')
-sys.exit(-1)
+logging.warning('No valid FileCheck executable; some tests may 
fail...')
+logging.warning('(Pass in a --filecheck argument to dotest.py)')
 
 if args.channels:
 lldbtest_config.channels = args.channels


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -311,8 +311,8 @@
 # The CMake build passes in a path to a working FileCheck binary.
 configuration.filecheck = os.path.abspath(args.filecheck)
 else:
-logging.error('No valid FileCheck executable; aborting...')
-sys.exit(-1)
+logging.warning('No valid FileCheck executable; some tests may fail...')
+logging.warning('(Pass in a --filecheck argument to dotest.py)')
 
 if args.channels:
 lldbtest_config.channels = args.channels
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Zachary Turner via lldb-commits]

Why is FileCheck missing in the first place?
On Thu, Oct 11, 2018 at 3:13 PM Vedant Kumar via Phabricator <
revi...@reviews.llvm.org> wrote:

> vsk created this revision.
> vsk added reviewers: stella.stamenova, zturner.
>
> This allows bots which haven't updated to pass in --filecheck to dotest.py
> to run more tests. FileCheck-dependent tests will continue to fail.
>
>
> https://reviews.llvm.org/D53175
>
> Files:
>   lldb/packages/Python/lldbsuite/test/dotest.py
>
>
> Index: lldb/packages/Python/lldbsuite/test/dotest.py
> ===
> --- lldb/packages/Python/lldbsuite/test/dotest.py
> +++ lldb/packages/Python/lldbsuite/test/dotest.py
> @@ -311,8 +311,8 @@
>  # The CMake build passes in a path to a working FileCheck binary.
>  configuration.filecheck = os.path.abspath(args.filecheck)
>  else:
> -logging.error('No valid FileCheck executable; aborting...')
> -sys.exit(-1)
> +logging.warning('No valid FileCheck executable; some tests may
> fail...')
> +logging.warning('(Pass in a --filecheck argument to dotest.py)')
>
>  if args.channels:
>  lldbtest_config.channels = args.channels
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Zachary Turner via Phabricator via lldb-commits]

zturner added a subscriber: vsk.
zturner added a comment.

Why is FileCheck missing in the first place?


https://reviews.llvm.org/D53175



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Vedant Kumar via lldb-commits]

It's probably not, it's just that the --filecheck arg is missing. I wrote 
things this way because I was advised that dotest.py shouldn't assume where 
FileCheck is on the filesystem.

vedant

> On Oct 11, 2018, at 3:22 PM, Zachary Turner  wrote:
> 
> Why is FileCheck missing in the first place?
> On Thu, Oct 11, 2018 at 3:13 PM Vedant Kumar via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> vsk created this revision.
> vsk added reviewers: stella.stamenova, zturner.
> 
> This allows bots which haven't updated to pass in --filecheck to dotest.py to 
> run more tests. FileCheck-dependent tests will continue to fail.
> 
> 
> https://reviews.llvm.org/D53175 
> 
> Files:
>   lldb/packages/Python/lldbsuite/test/dotest.py
> 
> 
> Index: lldb/packages/Python/lldbsuite/test/dotest.py
> ===
> --- lldb/packages/Python/lldbsuite/test/dotest.py
> +++ lldb/packages/Python/lldbsuite/test/dotest.py
> @@ -311,8 +311,8 @@
>  # The CMake build passes in a path to a working FileCheck binary.
>  configuration.filecheck = os.path.abspath(args.filecheck)
>  else:
> -logging.error('No valid FileCheck executable; aborting...')
> -sys.exit(-1)
> +logging.warning('No valid FileCheck executable; some tests may 
> fail...')
> +logging.warning('(Pass in a --filecheck argument to dotest.py)')
> 
>  if args.channels:
>  lldbtest_config.channels = args.channels
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

I spent a bit more time on this and I think I have a better idea of what's 
happening.

Vedant added functionality so that we can use FileCheck-style checks inside 
non-lit LLDB tests. Part of that change was to require a parameter --filecheck 
to be passed when calling the scripts (such as dotests.py). Also as part of 
that change, the CMakeFiles now propagate a default location for FileCheck as 
an input to the scripts, so that if you just run the tests by default using any 
of the generated outputs (for ninja, or make, or VS), they work correctly

The bots (at least some bots) don't just use the generated solutions to run the 
tests. There's some complicated logic in the zorg repo to create the list of 
parameters that are passed to the bots and they do not include the new 
--filecheck argument. So it's not individual bots that need to be updated, but 
the test harness. I haven't really looked at it before, so I'm still trying to 
understand it - especially how we would get FileCheck to any of the bots that 
run the tests remotely using lldbserver today.


https://reviews.llvm.org/D53175



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Vedant Kumar via lldb-commits]

> On Oct 11, 2018, at 3:30 PM, Stella Stamenova via Phabricator 
>  wrote:
> 
> stella.stamenova added a comment.
> 
> I spent a bit more time on this and I think I have a better idea of what's 
> happening.
> 
> Vedant added functionality so that we can use FileCheck-style checks inside 
> non-lit LLDB tests. Part of that change was to require a parameter 
> --filecheck to be passed when calling the scripts (such as dotests.py). Also 
> as part of that change, the CMakeFiles now propagate a default location for 
> FileCheck as an input to the scripts, so that if you just run the tests by 
> default using any of the generated outputs (for ninja, or make, or VS), they 
> work correctly
> 
> The bots (at least some bots) don't just use the generated solutions to run 
> the tests. There's some complicated logic in the zorg repo to create the list 
> of parameters that are passed to the bots and they do not include the new 
> --filecheck argument.

I did update zorg. See r342507 and r342559. Ah, is there a Windows-specific 
configuration I missed?


> So it's not individual bots that need to be updated, but the test harness. I 
> haven't really looked at it before, so I'm still trying to understand it - 
> especially how we would get FileCheck to any of the bots that run the tests 
> remotely using lldbserver today.

zorg/jenkins/build.py in https://github.com/llvm-mirror/zorg.git 
 might be relevant.

vedant

> 
> 
> https://reviews.llvm.org/D53175
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

The failing bots are not windows bots but Linux bots. It looks like you only 
updated the configurations for xcode.

I think the file that needs to be updated is:

zorg\buildbot\builders\LLDBBuilder.py


https://reviews.llvm.org/D53175



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I've been meaning to look at this - for the command line style "cd test; 
./dotest.py" approach, we could use similar logic to what 
packages/Python/lldbsuite/test/dotest.py does in getOutputPaths() where it 
finds an lldb binary in the "typical" location.  for an Xcode style build, this 
would be in a directory like ../llvm-build/Release+Asserts/x86_64/bin/FileCheck 
 where the build-style ("Release+Asserts") is going to vary but I'm guessing 
that the build configuration of the FileCheck binary picked is not important.


https://reviews.llvm.org/D53175



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

(obviously this would only be the behavior if a filecheck path was not 
specified)


https://reviews.llvm.org/D53175



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Vedant Kumar via lldb-commits]

Does this look reasonable to you? I'm not sure how to test this.

diff --git a/zorg/buildbot/builders/LLDBBuilder.py 
b/zorg/buildbot/builders/LLDBBuilder.py
index 5a1b2e87..62152924 100644
--- a/zorg/buildbot/builders/LLDBBuilder.py
+++ b/zorg/buildbot/builders/LLDBBuilder.py
@@ -270,6 +270,7 @@ def getLLDBTestSteps(f,
 compilerPath = compiler
 for arch in test_archs:
 DOTEST_OPTS=''.join(['--executable ' + bindir + '/lldb ',
+ '--filecheck ' + bindir + '/FileCheck ',
  '-A %s ' % arch,
  '-C %s ' % compilerPath,
  '-s lldb-test-traces-%s-%s ' % (compiler, 
arch),
@@ -819,6 +820,7 @@ def getLLDBxcodebuildFactory(use_cc=None,
   workdir=lldb_srcdir))
 DOTEST_OPTS = ' '.join(['--executable',
 '%(lldb_bindir)s/lldb',
+'%(lldb_bindir)s/FileCheck',
 '--framework', '%(lldb_bindir)s/LLDB.framework',
 '-A', 'x86_64',
 '-C', 'clang',

vedant

> On Oct 11, 2018, at 3:46 PM, Stella Stamenova via Phabricator 
>  wrote:
> 
> stella.stamenova added a comment.
> 
> The failing bots are not windows bots but Linux bots. It looks like you only 
> updated the configurations for xcode.
> 
> I think the file that needs to be updated is:
> 
> zorg\buildbot\builders\LLDBBuilder.py
> 
> 
> https://reviews.llvm.org/D53175
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

[Vedant Kumar via lldb-commits]

Can we get the Xcode build to generate lldb-dotest, just like the cmake build 
does? That way there's no need to duplicate build system logic into dotest.py. 
It would just live in one place, and we'd all invoke lldb-dotest the same way.

vedant

> On Oct 11, 2018, at 3:53 PM, Jason Molenda via Phabricator 
>  wrote:
> 
> jasonmolenda added a comment.
> 
> (obviously this would only be the behavior if a filecheck path was not 
> specified)
> 
> 
> https://reviews.llvm.org/D53175
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344323 - Remove references to source/Plugins/SymbolFile/NativePDB.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Thu Oct 11 17:53:55 2018
New Revision: 344323

URL: http://llvm.org/viewvc/llvm-project?rev=344323&view=rev
Log:
Remove references to source/Plugins/SymbolFile/NativePDB.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=344323&r1=344322&r2=344323&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Thu Oct 11 17:53:55 2018
@@ -210,7 +210,6 @@
2642FBAE13D003B400ED6808 /* CommunicationKDP.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 2642FBA813D003B400ED6808 /* 
CommunicationKDP.cpp */; };
964463EC1A330C0500154ED8 /* CompactUnwindInfo.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 964463EB1A330C0500154ED8 /* 
CompactUnwindInfo.cpp */; };
268900D513353E6F00698AC0 /* CompileUnit.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26BC7F1510F1B8EC00F91463 /* CompileUnit.cpp */; 
};
-   AF9BB7F8216EC7220093FA65 /* CompileUnitIndex.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = AF9BB7F5216EC7210093FA65 /* 
CompileUnitIndex.cpp */; };
265192C61BA8E905002F08F6 /* CompilerDecl.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 265192C51BA8E905002F08F6 /* CompilerDecl.cpp */; 
};
2657AFB71B86910100958979 /* CompilerDeclContext.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 2657AFB61B86910100958979 /* 
CompilerDeclContext.cpp */; };
268900D213353E6F00698AC0 /* CompilerType.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 49E45FAD11F660FE008F7B28 /* CompilerType.cpp */; 
};
@@ -587,8 +586,6 @@
4CA0C6CC20F929C700CFE6BB /* PDBLocationToDWARFExpression.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 4CA0C6CA20F929C600CFE6BB /* 
PDBLocationToDWARFExpression.cpp */; };
268900EE13353E6F00698AC0 /* PathMappingList.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 495BBACB119A0DBE00418BEA /* PathMappingList.cpp 
*/; };
2668A2EE20AF417D00D94111 /* PathMappingListTest.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 2668A2ED20AF417D00D94111 /* 
PathMappingListTest.cpp */; };
-   AF9BB7F6216EC7220093FA65 /* PdbIndex.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = AF9BB7F3216EC7210093FA65 /* PdbIndex.cpp */; };
-   AF9BB7F7216EC7220093FA65 /* PdbUtil.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = AF9BB7F4216EC7210093FA65 /* PdbUtil.cpp */; };
25420ED21A649D88009ADBCB /* PipeBase.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 25420ED11A649D88009ADBCB /* PipeBase.cpp */; };
2377C2F819E613C100737875 /* PipePosix.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = 2377C2F719E613C100737875 /* PipePosix.cpp */; };
268900EF13353E6F00698AC0 /* Platform.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 264A43BD1320BCEB005B4096 /* Platform.cpp */; };
@@ -915,7 +912,6 @@
4C7D48251F5099B2005314B4 /* SymbolFileDWARFDwoDwp.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 4C7D481F1F509964005314B4 /* 
SymbolFileDWARFDwoDwp.cpp */; };
4C7D48241F5099A1005314B4 /* SymbolFileDWARFDwp.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 4C7D481C1F509963005314B4 /* 
SymbolFileDWARFDwp.cpp */; };
9A2057121F3B824B00F6C293 /* SymbolFileDWARFTests.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 9A20570A1F3B81F300F6C293 /* 
SymbolFileDWARFTests.cpp */; };
-   AF9BB7F2216EC6EF0093FA65 /* SymbolFileNativePDB.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = AF9BB7F1216EC6EF0093FA65 /* 
SymbolFileNativePDB.cpp */; };
AF6335E21C87B21E00F7D554 /* SymbolFilePDB.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF6335E01C87B21E00F7D554 /* SymbolFilePDB.cpp 
*/; };
268900CE13353E5F00698AC0 /* SymbolFileSymtab.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 260C89DE10F57C5600BB2B04 /* 
SymbolFileSymtab.cpp */; };
268900E013353E6F00698AC0 /* SymbolVendor.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF94005711C03F6500085DB9 /* SymbolVendor.cpp */; 
};
@@ -1667,7 +1663,6 @@
964463ED1A330C1B00154ED8 /* CompactUnwindInfo.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
CompactUnwindInfo.h; path = include/lldb/Symbol/CompactUnwindInfo.h; sourceTree 
= ""; };
26BC7F1510F1B8EC00F91463 /* CompileUnit.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = CompileUnit.cpp; path = source/Symbol/CompileUnit.cpp; sourceTree = 
""; };
26BC7C5710F1B6E900F91463 /* CompileUnit.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileTy

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D53086#1261697, @aleksandr.urakov wrote:

> Thanks a lot for so detailed answer, it helps!
>
> So we need to parse a FPO program and to convert it in a DWARF expression 
> too. The problem here (in the DIA case) is that I don't know how to retrieve 
> the required FPO range (we have a symbol context when creating a variable, 
> but it seems that it doesn't contain enough information).


I think the `SymbolContext` should have enough information.  As long as you can 
find the `PDBSymbolFunction` for the current frame, that gives you the range of 
the function, and the `IDiaSymbol` for the variable should give you the 
important info like register, offset, etc.

> As for the raw PDB case, can the same `S_LOCAL` have several 
> `S_DEFRANGE_FRAMEPOINTER_REL` records for several ranges?  If so, then the 
> problem exists for the raw PDB case too, but there's a solution: we can 
> combine processing of all `S_DEFRANGE_FRAMEPOINTER_REL` records in the single 
> DWARF expression (and call the required FPO program depending on current 
> `eip`).

Not as far as I know.  There should be only 1.  But note that is not the only 
type of record that can appear, there are several others.  But basically there 
will be `S_LOCAL` followed by 1 record describing the location.

> The interesting moment here is that MSVC doesn't emit correct information for 
> locals. For the program `aaa.cpp`:
> 
>   void bar(char a, short b, int c) { }
>   
>   void __fastcall foo(short arg_0, float arg_1) {
> char loc_0 = 'x';
> double __declspec(align(8)) loc_1 = 0.5678;
> bar(1, 2, 3);
>   }
>   
>   int main(int argc, char *argv[]) {
> foo(, 0.1234);
> return 0;
>   }
> 
> 
> compiled with `cl /Zi /Oy aaa.cpp` we have the next disassembly of `foo`:
> 
>   pushebp
>   mov ebp, esp
>   and esp, 0FFF8h
>   sub esp, 10h
>   mov [esp+4], cx ; arg_0
>   mov byte ptr [esp+3], 'x' ; loc_0
>   movsd   xmm0, ds:__real@3fe22b6ae7d566cf
>   movsd   qword ptr [esp+8], xmm0 ; loc_1
>   push3   ; c
>   push2   ; b
>   push1   ; a
>   callj_?bar@@YAXDFH@Z ; bar(char,short,int)
>   add esp, 0Ch
>   mov esp, ebp
>   pop ebp
>   retn4
> 
> 
> but MSVC emits the next info about locals:
> 
>   296 | S_GPROC32 [size = 44] `foo`
> parent = 0, end = 452, addr = 0001:23616, code size = 53
> type = `0x1003 (void (short, float))`, debug start = 14, debug end = 
> 47, flags = has fp
>   340 | S_FRAMEPROC [size = 32]
> size = 16, padding size = 0, offset to padding = 0
> bytes of callee saved registers = 0, exception handler addr = 
> :
> local fp reg = VFRAME, param fp reg = EBP
> flags = has async eh | opt speed
>   372 | S_REGREL32 [size = 20] `arg_0`
> type = 0x0011 (short), register = ESP, offset = 4294967284
>   392 | S_REGREL32 [size = 20] `arg_1`
> type = 0x0040 (float), register = EBP, offset = 8
>   412 | S_REGREL32 [size = 20] `loc_1`
> type = 0x0041 (double), register = ESP, offset = 4294967288
>   432 | S_REGREL32 [size = 20] `loc_0`
> type = 0x0070 (char), register = ESP, offset = 4294967283
>   452 | S_END [size = 4]
>
> 
> so neither LLDB nor Visual Studio show valid values (except for `arg_1`).

Generally speaking, if you're using `/Oy` all bets are off and good luck :)

Interestingly, clang actually gets this right but we use a totally different 
CodeView record.

  $ llvm-pdbutil.exe dump -symbols -modi=0 foo-clang.pdb | grep -A 4 loc_1
   392 | S_LOCAL [size = 16] `loc_1`
 type=0x0041 (double), flags = none
   408 | S_DEFRANGE_FRAMEPOINTER_REL [size = 16]
 offset = -16, range = [0001:0075,+51)
 gaps = 2
  
  D:\src\llvmbuild\ninja-release-x64>

If you try to debug the same program in VS built with clang with the exact same 
command line, it will work.

I need to study the disassembly and FPO program of the cl.exe version some more 
to decide if the bug is in the compiler or the debugger, but I think the bug is 
in the compiler.

It's funny because our optimized debug info is not very good, so I'm surprised 
there's a case where we're better than MSVC here :)


https://reviews.llvm.org/D53086



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D53086#1263001, @zturner wrote:

> In https://reviews.llvm.org/D53086#1261697, @aleksandr.urakov wrote:
>
> > Thanks a lot for so detailed answer, it helps!
> >
> > So we need to parse a FPO program and to convert it in a DWARF expression 
> > too. The problem here (in the DIA case) is that I don't know how to 
> > retrieve the required FPO range (we have a symbol context when creating a 
> > variable, but it seems that it doesn't contain enough information).
>
>
> I think the `SymbolContext` should have enough information.  As long as you 
> can find the `PDBSymbolFunction` for the current frame, that gives you the 
> range of the function, and the `IDiaSymbol` for the variable should give you 
> the important info like register, offset, etc.


Do we have access to the current instruction pointer?  That's what you need to 
find the correct FPO record.


https://reviews.llvm.org/D53086



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits