hhellyer added inline comments.

================
Comment at: source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp:338
@@ -315,4 +337,3 @@
     // truncated.
-    error.SetErrorString("address is not in a known range");
     return error;
 }
----------------
amccarth wrote:
> hhellyer wrote:
> > Asking for an address outside a known range is not actually an error, you 
> > just get back an unmapped range that specifies how far it is to the next 
> > mapped range.
> > The original discussion about that happened here: 
> > https://reviews.llvm.org/D21751 - GetMemoryRegionInfo should only need to 
> > return an error if it is unimplemented. (I should probably have referenced 
> > that in the summary.)
> Got it.  Thanks for the explanation!
> 
> Given that, should the condition on 284 ("the mini dump contains no memory 
> range information") actually return an error or just an unmapped range?
I thought the most likely cause of having no memory info would be not passing 
the MiniDumpWithFullMemoryInfo flag to MiniDumpWriteDump will create a dump 
without the MemoryInfoListStream present. Saying that memory range information 
from a dump like that is "unsupported" seemed like the right option. I think 
MiniDumpWithFullMemoryInfo is part of the default set of flags used when you 
create a dump through task manager so most dumps should have the stream.

I won't land the patch immediately, I'm happy to change it if you'd prefer it 
to return an unmapped range. I think it would have to be one unmapped range 
that ran from load_addr to LLDB_INVALID_ADDRESS to match the expected behaviour.


https://reviews.llvm.org/D22352



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

Reply via email to