<div dir="ltr"><span style="color:rgb(33,33,33)">4ad5334bfcff803f3765e444785b8f</span><span style="color:rgb(33,33,33)">9d3a73c683: Don't pass a null StringRef. simple.</span><div><span style="color:rgb(33,33,33)">f7b079263a751fdf3adea8e549803a</span><span style="color:rgb(33,33,33)">af92d465f8: Maybe fix it instead, as the comment suggests?</span> <span style="color:rgb(33,33,33)"><br></span></div><div><span style="color:rgb(33,33,33)">f3647763b02ddef65c6244f91939d9</span><span style="color:rgb(33,33,33)">97f7733ecd: Probably fine since you don't have control over the code of the plugin.</span></div><div><span style="color:rgb(33,33,33)">f67e0b92fbd9e0bc0267ae5210478c</span><span style="color:rgb(33,33,33)">85a44b8afc: Probably fine since you're at the mercy of the system</span><span style="color:rgb(33,33,33)"><br></span></div><div><font color="#212121">0ac65423e2ba99a42b246d191520ff13bdca5cb0: There's no indication here why the TypeSP might be empty. Is it bad debug info? If so it should be easy to construct a test. If the answer is "I don't know", then this doesn't really fix anything.</font><br></div><div><span style="color:rgb(33,33,33)">7043340bc58b0d751fcf66001f62cb</span><span style="color:rgb(33,33,33)">f0d9527623: Probably fine, debug info is bad.</span></div><div><span style="color:rgb(33,33,33)">d7019a8d5ccd5dfdb79d74312ef449</span><span style="color:rgb(33,33,33)">b734627ec3: Seems easy to write a test for. That said, I would argue that the function should return a reference and the person calling the function ensure that we *do* have a TypeSystem for the specified language. There are plenty of places where the function is already called with the knowledge that a TypeSystem exists. In those cases, the function absolutely should assert. By moving the responsibility up and clearly documenting it, you allow</span><font color="#212121"><br></font></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 12, 2017 at 1:23 PM Jim Ingham <<a href="mailto:jingham@apple.com">jingham@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Huh, yeah I don't see any of these bugs as making your point. These are all fixes so I don't see how they argue that we're willfully ignoring anything. To the extent that they deal with asserts they fixes for lldb not remembering that it has to tread carefully to avoid triggering other asserts elsewhere.<br>
<br>
Jim<br>
<br>
> On Sep 12, 2017, at 1:17 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
><br>
> (Some of those look like correct fixes btw, since they deal with user input)<br>
><br>
> On Tue, Sep 12, 2017 at 1:16 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
> On Tue, Sep 12, 2017 at 1:04 PM Jim Ingham <<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>> wrote:<br>
><br>
> I don't see any evidence for lldb suffering from "a huge class of bugs that we are willfully ignoring..." particularly ones that would be easily caught if we just had more asserts. Can you give some examples?<br>
><br>
> Probably all of these, for starters:<br>
><br>
> D:\src\llvm-mono>git log --grep "Don't crash" lldb<br>
> commit 4ad5334bfcff803f3765e444785b8f9d3a73c683<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Mon Jul 24 16:47:04 2017 +0000<br>
><br>
> Don't crash when hostname is empty. StringRef will assert and kill your program.<br>
><br>
> commit f7b079263a751fdf3adea8e549803aaf92d465f8<br>
> Author: Sean Callanan <<a href="mailto:scallanan@apple.com" target="_blank">scallanan@apple.com</a>><br>
> Date: Fri Aug 26 18:12:39 2016 +0000<br>
><br>
> Don't crash when trying to capture persistent variables in a block.<br>
><br>
> Reports an error instead. We can fix this later to make persistent variables<br>
> work, but right now we hit an LLVM assertion if we get this wrong.<br>
><br>
> <rdar://problem/27770298><br>
><br>
> commit f3647763b02ddef65c6244f91939d997f7733ecd<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Mon May 16 20:07:38 2016 +0000<br>
><br>
> Don't crash when OS plug-in returns None from any of the functions we might call.<br>
><br>
> <rdar://problem/24489419><br>
><br>
> commit f67e0b92fbd9e0bc0267ae5210478c85a44b8afc<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Thu May 12 22:36:47 2016 +0000<br>
><br>
> Don't crash when a process' task port goes bad.<br>
><br>
> <rdar://problem/26256049><br>
><br>
> commit 0ac65423e2ba99a42b246d191520ff13bdca5cb0<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Tue Mar 15 21:58:28 2016 +0000<br>
><br>
> Don't crash if the TypeSP is empty.<br>
><br>
> commit 7043340bc58b0d751fcf66001f62cbf0d9527623<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Fri Feb 12 00:07:16 2016 +0000<br>
><br>
> Don't crash if we have a DIE that has a DW_AT_ranges attribute and yet the SymbolFileDWARF doesn't have a DebugRanges. If this happens print a nice error message to prompt the user to file a bug and attach the offending DWARF file so we can get the correct compiler fixed.<br>
><br>
> <rdar://problem/24458016><br>
><br>
> commit 0d1591d3b74d518b9edd7482f65976092c14e951<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Wed Oct 28 20:49:34 2015 +0000<br>
><br>
> Don't crash when opening a fuzzed mach-o file that has bad dyld trie data.<br>
><br>
> <rdar://problem/21991784><br>
><br>
> commit d7019a8d5ccd5dfdb79d74312ef449b734627ec3<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Fri Aug 14 23:15:48 2015 +0000<br>
><br>
> Don't crash if we don't have a type system for a language.<br>
><br>
> commit b5838b5a4870ba8f620e7a5733038f02f45b1a78<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Thu Aug 13 23:16:15 2015 +0000<br>
><br>
> Don't crash when we have a .a file that contains an object with a 16 character name. Any calls to std::string::erase must be bounds checked.<br>
><br>
> <rdar://problem/22260988><br>
><br>
> commit 4234fd5de6adb471728df83670ebbe0ae3c7ee68<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Tue Aug 11 21:01:32 2015 +0000<br>
><br>
> Don't crash if the file we want to touch doesn't exist.<br>
><br>
> commit b385164c734997af6367271d33dc1c4618bfb754<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Mon Jul 13 22:08:16 2015 +0000<br>
><br>
> Don't crash if we are unable to get the member type.<br>
><br>
> <rdar://problem/21624447><br>
><br>
> commit a5deec8dc9a9e7fd977049f6fb5977796906e8ca<br>
> Author: Sean Callanan <<a href="mailto:scallanan@apple.com" target="_blank">scallanan@apple.com</a>><br>
> Date: Thu May 28 20:06:40 2015 +0000<br>
><br>
> Don't crash if we don't have a process and need<br>
> to check for alternate manglings.<br>
><br>
> commit 4427526ffa55675b623702452ff6e13c33c79763<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Fri May 15 22:31:18 2015 +0000<br>
><br>
> Don't crash if we have bad debug info that has a DW_TAG_inheritance with a bad DW_AT_type reference. Emit an error with instructions to file a bug.<br>
><br>
> <rdar://problem/20944860><br>
><br>
> commit 4506ae8792a5b726133a58d1bc887313bceccd93<br>
> Author: Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>><br>
> Date: Fri May 15 22:20:29 2015 +0000<br>
><br>
> Don't crash if a function has no name by calling 'strcmp(name, "main")'.<br>
><br>
> <rdar://problem/20925061><br>
><br>
<br>
</blockquote></div>