Re: [lldb-dev] [cfe-dev] "devirtualizing" files in the VFS

2018-11-27 Thread Jonas Devlieghere via lldb-dev 
Hi Sam,

Does extending the status with a path sound reasonable? This would work similar 
to the current Name field, which is controlled by UseExternalName. 

Please let me know what you think.

Thanks,
Jonas 

> On Nov 15, 2018, at 10:10 AM, Jonas Devlieghere via cfe-dev 
>  wrote:
> 
> 
>> On Nov 15, 2018, at 3:34 AM, Whisperity > > wrote:
>> 
>> I am really not sure if adding real file system functionality strictly into 
>> the VFS is a good approach. This "ExternalFileSystem" thing sounds weird to 
>> me.
> 
> The `ExternalFileSystem` was an attempt to provide a more limited interface 
> while exposing the "external" path in a way that made sense for the 
> RedirectingFileSystem. Like Sam said in the review it's not great because it 
> only does half of the work. 
> 
>> Does LLDB need to *write* the files through the VFS? I'm not sure perhaps a 
>> "WritableVFS" could be implemented, or the necessary casting/conversion 
>> options.
> 
> Most likely yes because of LLDB's design that abstracts over flies without 
> prior knowledge about whether they'd only get read or written. However 
> wouldn't it suffer form the exact same problems? 
> 
>> In case:
>>  - there is a real path behind the file --- you could spawn an 
>> llvm::RealFileSystem (the fqdn might not be this after the migration patch!) 
>> and use that to obtain the file's buffer.
> 
> I'm not sure I follow what you have in mind here. Can you give a little more 
> detail?
> 
>> How can you be sure the file actually exists on the FS? That's what the VFS 
>> should be all about, hiding this abstraction... if you *are* sure it exists, 
>> or want to make sure, you need to pull the appropriate realFS from the VFS 
>> Overlay (most tools have an overlay of a memoryFS above the realFS).
> 
> That makes sense, for LLDB's use case we would be happy having just a real or 
> redirecting filesystem (with fall through). 
> 
>> What I am not sure about is extending the general interface in a way that it 
>> caters to a particular (or half of a particular) use case.
> 
> I totally understand this sentiment but I don't think that's totally fair. 
> Finding files in different locations is an important feature of the VFS, when 
> it was introduced in 2014 this was the only use case. The "devirtualization" 
> aspect is unfortunate because native IO. 
> 
>> For example, in CodeCompass, we used a custom VFS implementation that 
>> "hijacked" the overlay and included itself between the realFS and the 
>> memoryFS. It obtains files from the database!
>> 
>> See:
>> https://github.com/Ericsson/CodeCompass/blob/a1a7b10e3a9e2e4f493135ea68566cee54adc081/plugins/cpp_reparse/service/src/databasefilesystem.cpp#L191-L224
>>  
>> 
>> 
>> These files *do not necessarily* (in 99% of the cases, not at all) exist on 
>> the hard drive at the moment of the code wanting to pull the file, hence why 
>> we implemented this to give the file source buffer from DB. The ClangTool 
>> that needs this still gets the memoryFS for its own purposes, and for the 
>> clang libraries, the realFS is still under there.
>> 
>> Perhaps the "Status" type could be extended to carry extra information? 
>> https://github.com/Ericsson/CodeCompass/blob/a1a7b10e3a9e2e4f493135ea68566cee54adc081/plugins/cpp_reparse/service/src/databasefilesystem.cpp#L85-L87
>>  
>> 
> 
> This sounds like an interesting idea. We already have the option to expose 
> the external name here, would it be reasonable to also expose the external 
> path here? (of course being an optional)
> 
>> 
>> Sam McCall via cfe-dev > > ezt írta (időpont: 2018. nov. 15., Cs, 
>> 12:02):
>> I'd like to get some more perspectives on the role of the VirtualFileSystem 
>> abstraction in llvm/Support.
>> (The VFS layer has recently moved from Clang to LLVM, so crossposting to 
>> both lists)
>> 
>> https://reviews.llvm.org/D54277  proposed 
>> adding a function to VirtualFileSystem to get the underlying "real file" 
>> path from a VFS path. LLDB is starting to use VFS for some filesystem 
>> interactions, but wants/needs to keep using native IO (FILE*, file 
>> descriptors) for others. There's some more context/discussion in the review.
>> 
>> My perspective is coloured by work on clang tooling, clangd etc. There we 
>> rely on VFS to ensure code (typically clang library code) works in a variety 
>> of environments, e.g:
>> in an IDE the edited file is consistently used rather than the one on disk
>> clang-tidy checks work on a local codebase, but our code review tool also 
>> runs them as a service
>> This works because all IO goes through the VFS, so VFSes ar

[lldb-dev] [Bug 39727] Support for breakpad symbols in LLDB

2018-11-27 Thread via lldb-dev 
https://bugs.llvm.org/show_bug.cgi?id=39727

David Finkelstein  changed:

   What|Removed |Added

   Assignee|lldb-dev@lists.llvm.org |lab...@google.com

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] [cfe-dev] "devirtualizing" files in the VFS

2018-11-27 Thread Sam McCall via lldb-dev 
On Tue, Nov 27, 2018 at 7:01 PM Jonas Devlieghere 
wrote:

> Hi Sam,
>
> Does extending the status with a path sound reasonable? This would work
> similar to the current Name field, which is controlled by UseExternalName.
>
> Please let me know what you think.
>
> Thanks,
> Jonas
>

Design-wise, adding a second Name field to Status doesn't seem
significantly different than adding a "getOtherName" field to VFS.
I don't think it's really reasonable to have this in the VFS interfaces,
because using it in any way means you're coupled to particular
implementations.

Implementation-wise, it seems the only way to do that would be to add an
extra string to Status, and some applications probably keep a lot of Status
objects. So it doesn't seem like an improvement vs the VFS method.

Overall I think if LLDB wants to wants to break abstraction and use the
native filesystem sometimes, it should have concrete private
implementations with wider interfaces, and bridge to common code by having
them implement the VFS interfaces. The contract of VFS may have been "real
filesystem with some redirected files", but I don't think it has been for
several years, and people rely on this.

I'm not an owner here though. I don't really know how these decisions get
made, and would like to hear more opinions.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

[lldb-dev] [Bug 39816] New: lldb doesn't show a file of line entry for big project.

2018-11-27 Thread via lldb-dev 
https://bugs.llvm.org/show_bug.cgi?id=39816

Bug ID: 39816
   Summary: lldb doesn't show a file of line entry for big
project.
   Product: lldb
   Version: 7.0
  Hardware: All
OS: All
Status: NEW
  Severity: enhancement
  Priority: P
 Component: All Bugs
  Assignee: lldb-dev@lists.llvm.org
  Reporter: frozenrain@gmail.com
CC: llvm-b...@lists.llvm.org

Created attachment 21167
  --> https://bugs.llvm.org/attachment.cgi?id=21167&action=edit
lldb.patch1

You can see Entry struct in lldb/include/lldb/Symbol/LineTable.h

file_idx type is NOT uint16_t, provide 11 bits. (... uint16_t file_idx : 11,
...)

our object has many header files more than 2048..

if file_idx is 2691..
file_idx = 2691 (0b10101011) <-- 12bits

file_idx provides 11 bits.
so, file_idx variable changed from 2691(0b10101011) to 643(0b0101011).

I think this is a bug.
Fix it. Please.


you can patch following code(attachment files).
please review and submit it.
I don't know this struct data needs serialization..

* patch1
diff --git a/include/lldb/Symbol/LineTable.h b/include/lldb/Symbol/LineTable.h
index a6d4364b9..0300455ec 100644
--- a/include/lldb/Symbol/LineTable.h
+++ b/include/lldb/Symbol/LineTable.h
@@ -311,8 +311,7 @@ protected:
  ///number information.
 uint16_t column; ///< The column number of the source line, or zero if
there
  ///is no column information.
-uint16_t file_idx : 11, ///< The file index into CompileUnit's file table,
-///or zero if there is no file information.
+uint32_t file_idx : 27,///or zero if there is no file information.
 is_start_of_statement : 1, ///< Indicates this entry is the beginning
of
///a statement.
 is_start_of_basic_block : 1, ///< Indicates this entry is the
beginning



* patch2
diff --git a/include/lldb/Symbol/LineTable.h b/include/lldb/Symbol/LineTable.h
index a6d4364b9..793c3331c 100644
--- a/include/lldb/Symbol/LineTable.h
+++ b/include/lldb/Symbol/LineTable.h
@@ -311,21 +311,20 @@ protected:
  ///number information.
 uint16_t column; ///< The column number of the source line, or zero if
there
  ///is no column information.
-uint16_t file_idx : 11, ///< The file index into CompileUnit's file table,
-///or zero if there is no file information.
-is_start_of_statement : 1, ///< Indicates this entry is the beginning
of
-   ///a statement.
-is_start_of_basic_block : 1, ///< Indicates this entry is the
beginning
- ///of a basic block.
-is_prologue_end : 1, ///< Indicates this entry is one (of possibly
many)
- ///where execution should be suspended for an
entry
- ///breakpoint of a function.
-is_epilogue_begin : 1, ///< Indicates this entry is one (of possibly
-   ///many) where execution should be suspended
for
-   ///an exit breakpoint of a function.
-is_terminal_entry : 1; ///< Indicates this entry is that of the first
-   ///byte after the end of a sequence of target
-   ///machine instructions.
+uint16_t file_idx;///or zero if there is no file information.
+bool is_start_of_statement; ///< Indicates this entry is the beginning of
+///a statement.
+bool is_start_of_basic_block; ///< Indicates this entry is the beginning
+  ///of a basic block.
+bool is_prologue_end; ///< Indicates this entry is one (of possibly many)
+  ///where execution should be suspended for an entry
+  ///breakpoint of a function.
+bool is_epilogue_begin; ///< Indicates this entry is one (of possibly
+///many) where execution should be suspended for
+///an exit breakpoint of a function.
+bool is_terminal_entry; ///< Indicates this entry is that of the first
+///byte after the end of a sequence of target
+///machine instructions.
   };

   struct EntrySearchInfo {

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev