[PATCH] D88288: [Archive] Don't throw away errors for malformed archive members

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.


================
Comment at: llvm/lib/Object/SymbolicFile.cpp:51
       return IRObjectFile::create(Object, *Context);
-    LLVM_FALLTHROUGH;
-  case file_magic::unknown:
-  case file_magic::archive:
-  case file_magic::coff_cl_gl_object:
-  case file_magic::macho_universal_binary:
-  case file_magic::windows_resource:
-  case file_magic::pdb:
-  case file_magic::minidump:
-  case file_magic::tapi_file:
     return errorCodeToError(object_error::invalid_file_type);
   case file_magic::elf:
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > I think the `Content` is always non-null here now?
> > Yeah, I think this can be simplified. I'm not going to have a chance to update the patch again today, but will fix in the commit, if you're otherwise happy.
> > 
> > It will become:
> > ```
> >   case file_magic::bitcode:
> >     return IRObjectFile::create(Object, *Context);
> > ```
> The rest looks fine to me. Please wait to see @MaskRay/@rupprecht opinions too
`Context` is always non-null. This is however a hidden fact only clear after I read all the callers..


================
Comment at: llvm/test/Object/archive-unknown-filetype.test:5
+
+# RUN: touch %t
+# RUN: rm -f %t.a
----------------
Using `echo something > %t` can be more robust. `touch %t` does not update the file content. If %t preexists as a different file type, this may fail spuriously.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88288/new/

https://reviews.llvm.org/D88288