jimingham wrote: > > For the first half, maybe you would avoid crashes by keeping everything > > around, but you could end up having a global variable that got its type > > from one Module and then you replace the module with another and get a > > local variable out of them and now you have some stale types and some new > > ones... That just seems like leading towards confusion even if not crashes. > > we want to replace placeholders with real modules. There will be no global > variables or other things around for these. And if the module is kept around, > nothing will crash. >
That's what YOU want to do, but the command in no way requires that. That's why my complaint was that you were adding something sharp-edged, not that you were adding something that couldn't be made to work in some circumstances. We try to avoid sharp-edged commands in lldb... If you can figure out some way to make `target modules replace` refuse to do the job except in the case where the original module hasn't really been used at all in the current process session, this would be all good. But I don't see how you can do that... > Not sure what stale types you are talking about here. That happens with > modules being replaced when you re-launch your target every day. We don't > have crashes related to those. This is not much different than restarting a > debug session with a freshly built binary. We don't keep frames, or their variables, or VO's representing global variables, etc. across runs, so this isn't an apt comparison. > > > It sounds to me like you are trying to model lazy module creation, so it > > seems like it would be better to do that explicitly rather than have the > > user have to juggle this and hope they didn't cause any confusions. > > We aren't. We are just trying to deal with millions of core files that we > have everyday and we don't always have symbols as they live for a small > amount of time before they get removed from our symbol server. Sometimes > people still have symbols in their personal directories and they send us the > symbol file for a bug report. Either way, it is useful for us to be able to > replace a placeholder with a valid file. > > > You could for instance make a placeholder Module class that can answer the > > sort of questions the ModuleSpec provides answers to. If you ask it any > > questions that might require more information, the Placeholder Module could > > use the ModuleLocator to realize the actual module, and hold that as a > > private module in the Placeholder and forward all questions it couldn't > > answer to the privately held module. Everything you got from the Module > > would be from one source, and so would stay consistent. If another Target > > came along and asked for the same UUID it wouldn't be a crap-shoot what it > > got. It would get the placeholder Module from the global module cache and > > everything would stay orderly. > > So your idea is to have the placeholder module, every time it needs something > that it can't supply, call the ModuleLocator over and over until it finally > says yes, instead of a user being able to just replace the module if the user > knows they want to? I don't understand this comment. Why would the ModuleLocator have to be called "over and over till it finally says yes"? Why wouldn't it be some algorithm that succeeds or fails? If you knew you had failed, you'd just cache that fact and stick with the placeholder? Anyway, presumably the module locator is using the same method the user would to produce the artifact that they are then going to "replace" the original one with. So the experience in both cases would be as good as the module locator was, just that in the `replace` workflow they'd have to do that preparatory work by hand. > > We can keep this as an internal patch if no one wants this, but it seemed > like a good thing to upstream as it has been useful for us. https://github.com/llvm/llvm-project/pull/214576 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
