[PATCH] D17104: [VFS] Drop path traversal assertion

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 12 11:43:36 PST 2016


bruno added a comment.

> I'm not sure I understand how the 2nd path would cover the realpath case.  It is just an entry for the realpath itself; that doesn't help if the client looks up "/install-dir/lib/clang/3.8.0" directly (not just from removing dots from bin/..).


What I was suggestting is this:

'type': 'directory',
'name': "/install-dir/lib/clang/3.8.0/include",
'contents': [

  ...
  {
    'type': 'file',
    'name': "altivec.h",
    'external-contents': "<path_to_cache>/private/tmp/a/b/lib/clang/3.8.0/include/altivec.h"
  },

'type': 'directory',
'name': "/private/tmp/a/b/lib/clang/3.8.0/include",
'contents': [

  ...
  {
    'type': 'file',
    'name': "altivec.h",
    'external-contents': "<path_to_cache>/private/tmp/a/b/lib/clang/3.8.0/include/altivec.h"
  },
  ...

The second case is here to address what you mentioned in the first comment "looking up the path *without* the .. won't work, which means anything that looks up a realpath will fail". But thinking more about it, I don't see how it's useful to have this second entry since the VFS will only be requested for "/install-dir/bin/../lib/clang/3.8.0/include" anyway. What about we stick to 1st path? This should solve the ".." issue and at the same time synchronise with the virtual path lookup if we use remove_dots() over there too. Am I missing something here?

> > Correct me if I'm wrong, but do you suggest we have a new entry type in the YAML file for declaring symlinks? Right now, we handle symlinks by copying them out as real files inside the VFS. However, adding extra "name" entries to map for real paths for such symlinks inside the YAML should do it.

> 

> 

> Yes, that's what I'm suggesting.


Is there any real reason for this additional logic? Why an regular additional entry in the YAML file isn't enough?
BTW, there are two cases for symlinks:

(A) The above where the symlink is a path component. Unless I missed something, this could be solved with only one path entry, like discussed above.
(B) The symlink is the final header itself, this could be solved without any extra logic by duplicating the entries while maintaining only one copy in the .cache/vfs dir, example:

Take the symlink:
/usr/include/pthread.h -> pthread/pthread.h

'type': 'directory',
'name': "/usr/include/",
'contents': [

  ...
  {
    'type': 'file',
    'name': "pthread.h",
    'external-contents': "<path_to_cache>/usr/include/pthread/pthread.h"
  },

...
'type': 'directory',
'name': "/usr/include/pthread",
'contents': [

  ...
  {
    'type': 'file',
    'name': "pthread.h",
    'external-contents': "<path_to_cache>/usr/include/pthread/pthread.h"
  },
  ...


http://reviews.llvm.org/D17104





More information about the cfe-commits mailing list