November 2021 - Wine-Devel - list.winehq.org

[PATCH 2/5] winegstreamer: Implement IWMReaderAdvanced::SetStreamsSelected().

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index b7b96c8b884..ced02d454a2 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -85,7 +85,12 @@ static DWORD WINAPI stream_thread(void *arg) for (i = 0; i < stream_count; ++i) { - hr = wm_reader_get_stream_sample(&reader->reader.streams[i], &sample, &pts, &duration, &flags); + struct wm_stream *stream = &reader->reader.streams[i]; + + if (stream->selection == WMT_OFF) + continue; + + hr = wm_reader_get_stream_sample(stream, &sample, &pts, &duration, &flags); if (hr == S_OK) { if (reader->user_clock) @@ -441,12 +446,15 @@ static HRESULT WINAPI WMReaderAdvanced_GetManualStreamSelection(IWMReaderAdvance return E_NOTIMPL; } -static HRESULT WINAPI WMReaderAdvanced_SetStreamsSelected(IWMReaderAdvanced6 *iface, WORD stream_count, - WORD *stream_numbers, WMT_STREAM_SELECTION *selections) +static HRESULT WINAPI WMReaderAdvanced_SetStreamsSelected(IWMReaderAdvanced6 *iface, + WORD count, WORD *stream_numbers, WMT_STREAM_SELECTION *selections) { - struct async_reader *This = impl_from_IWMReaderAdvanced6(iface); - FIXME("(%p)->(%d %p %p)\n", This, stream_count, stream_numbers, selections); - return E_NOTIMPL; + struct async_reader *reader = impl_from_IWMReaderAdvanced6(iface); + + TRACE("reader %p, count %u, stream_numbers %p, selections %p.\n", + reader, count, stream_numbers, selections); + + return wm_reader_set_streams_selected(&reader->reader, count, stream_numbers, selections); } static HRESULT WINAPI WMReaderAdvanced_GetStreamSelected(IWMReaderAdvanced6 *iface, WORD stream_num, -- 2.33.0

Nov. 10, 2021

[PATCH 1/5] winegstreamer: Implement IWMSyncReader::SetStreamsSelected().

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 3 ++ dlls/winegstreamer/wm_reader.c | 48 ++++++++++++++++++++++++++++++ dlls/winegstreamer/wm_syncreader.c | 21 ++++++++----- 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 9674ee35052..c7c72ab65fc 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -123,6 +123,7 @@ struct wm_stream WORD index; bool eos; struct wg_format format; + WMT_STREAM_SELECTION selection; }; struct wm_reader @@ -173,5 +174,7 @@ HRESULT wm_reader_open_stream(struct wm_reader *reader, IStream *stream); void wm_reader_seek(struct wm_reader *reader, QWORD start, LONGLONG duration); HRESULT wm_reader_set_output_props(struct wm_reader *reader, DWORD output, IWMOutputMediaProps *props); +HRESULT wm_reader_set_streams_selected(struct wm_reader *reader, WORD count, + const WORD *stream_numbers, const WMT_STREAM_SELECTION *selections); #endif /* __GST_PRIVATE_INCLUDED__ */ diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index bcae50e5d1e..6b29415e363 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -1413,6 +1413,7 @@ static HRESULT init_stream(struct wm_reader *reader, QWORD file_size) stream->wg_stream = wg_parser_get_stream(reader->wg_parser, i); stream->reader = reader; stream->index = i; + stream->selection = WMT_ON; wg_parser_stream_get_preferred_format(stream->wg_stream, &stream->format); if (stream->format.major_type == WG_MAJOR_TYPE_AUDIO) { @@ -1739,6 +1740,9 @@ HRESULT wm_reader_get_stream_sample(struct wm_stream *stream, struct wg_parser_event event; struct buffer *object; + if (stream->selection == WMT_OFF) + return NS_E_INVALID_REQUEST; + if (stream->eos) return NS_E_NO_MORE_SAMPLES; @@ -1824,6 +1828,50 @@ void wm_reader_seek(struct wm_reader *reader, QWORD start, LONGLONG duration) LeaveCriticalSection(&reader->cs); } +HRESULT wm_reader_set_streams_selected(struct wm_reader *reader, WORD count, + const WORD *stream_numbers, const WMT_STREAM_SELECTION *selections) +{ + struct wm_stream *stream; + WORD i; + + if (!count) + return E_INVALIDARG; + + EnterCriticalSection(&reader->cs); + + for (i = 0; i < count; ++i) + { + if (!(stream = wm_reader_get_stream_by_stream_number(reader, stream_numbers[i]))) + { + LeaveCriticalSection(&reader->cs); + WARN("Invalid stream number %u; returning NS_E_INVALID_REQUEST.\n", stream_numbers[i]); + return NS_E_INVALID_REQUEST; + } + } + + for (i = 0; i < count; ++i) + { + stream = wm_reader_get_stream_by_stream_number(reader, stream_numbers[i]); + stream->selection = selections[i]; + if (selections[i] == WMT_OFF) + { + TRACE("Disabling stream %u.\n", stream_numbers[i]); + wg_parser_stream_disable(stream->wg_stream); + } + else if (selections[i] == WMT_ON) + { + if (selections[i] != WMT_ON) + FIXME("Ignoring selection %#x for stream %u; treating as enabled.\n", + selections[i], stream_numbers[i]); + TRACE("Enabling stream %u.\n", stream_numbers[i]); + wg_parser_stream_enable(stream->wg_stream, &stream->format); + } + } + + LeaveCriticalSection(&reader->cs); + return S_OK; +} + void wm_reader_init(struct wm_reader *reader, const struct wm_reader_ops *ops) { reader->IWMHeaderInfo3_iface.lpVtbl = &header_info_vtbl; diff --git a/dlls/winegstreamer/wm_syncreader.c b/dlls/winegstreamer/wm_syncreader.c index fff048df06e..3e980c59601 100644 --- a/dlls/winegstreamer/wm_syncreader.c +++ b/dlls/winegstreamer/wm_syncreader.c @@ -83,8 +83,8 @@ static HRESULT WINAPI WMSyncReader_GetNextSample(IWMSyncReader2 *iface, DWORD *flags, DWORD *output_number, WORD *ret_stream_number) { struct sync_reader *reader = impl_from_IWMSyncReader2(iface); + HRESULT hr = NS_E_NO_MORE_SAMPLES; struct wm_stream *stream; - HRESULT hr; WORD i; TRACE("reader %p, stream_number %u, sample %p, pts %p, duration %p," @@ -104,8 +104,12 @@ static HRESULT WINAPI WMSyncReader_GetNextSample(IWMSyncReader2 *iface, for (i = 0; i < reader->reader.stream_count; ++i) { WORD index = (i + reader->last_read_stream + 1) % reader->reader.stream_count; + struct wm_stream *stream = &reader->reader.streams[index]; - hr = wm_reader_get_stream_sample(&reader->reader.streams[index], sample, pts, duration, flags); + if (stream->selection == WMT_OFF) + continue; + + hr = wm_reader_get_stream_sample(stream, sample, pts, duration, flags); if (hr == S_OK) { if (output_number) @@ -296,12 +300,15 @@ static HRESULT WINAPI WMSyncReader_SetReadStreamSamples(IWMSyncReader2 *iface, W return E_NOTIMPL; } -static HRESULT WINAPI WMSyncReader_SetStreamsSelected(IWMSyncReader2 *iface, WORD stream_count, - WORD *stream_numbers, WMT_STREAM_SELECTION *selections) +static HRESULT WINAPI WMSyncReader_SetStreamsSelected(IWMSyncReader2 *iface, + WORD count, WORD *stream_numbers, WMT_STREAM_SELECTION *selections) { - struct sync_reader *This = impl_from_IWMSyncReader2(iface); - FIXME("(%p)->(%d %p %p): stub!\n", This, stream_count, stream_numbers, selections); - return S_OK; + struct sync_reader *reader = impl_from_IWMSyncReader2(iface); + + TRACE("reader %p, count %u, stream_numbers %p, selections %p.\n", + reader, count, stream_numbers, selections); + + return wm_reader_set_streams_selected(&reader->reader, count, stream_numbers, selections); } static HRESULT WINAPI WMSyncReader2_SetRangeByTimecode(IWMSyncReader2 *iface, WORD stream_num, -- 2.33.0

Nov. 10, 2021

Re: [PATCH vkd3d 2/5] vkd3d-shader/hlsl: Perform a copy propagation pass.

by Zebediah Figura (she/her)

On 11/10/21 03:57, Giovanni Mascellani wrote: > Hi, > > On 09/11/21 23:21, Zebediah Figura wrote: >> Overall this patch looks a lot better than I was afraid of. It's a lot >> of code that's intimidating to review, but once you ignore the rbtree >> boilerplate it's simple enough and seems about in line with what I >> expect. There's quite a few things that I think can be simplified, but >> the basic structure seems sound, so nice work. > > Great, that's good! Thanks for your review, I think most if not all of > the points you wrote should be easy to fix. > >> The CF part is probably going to be a lot trickier, but fortunately I >> think that this patch alone is the one that really matters. Frankly, >> if we were to take this patch, and then a second patch that does >> copy-prop on interior CF blocks with a fresh copy_propagation_state(), >> we might even cover enough that it's not even worrying about CF... > > Mmh, note that you have to handle control flow in some way or another, > you cannot just ignore it. This patch doesn't know about control flow, > but the only way it can do it is by bailing out at the first control > flow instruction in the program. It's not enough to ignore the control > flow block and resume processing after it, because the inner block might > invalidate some of your knowledge about the program state (in 3/5 patch > this is done by copy_propagation_invalidate_from_block). Right. I should have said to invalidate the whole copyprop state both when entering a CF block and when leaving one. The point is that we can do copyprop within every BB, not just the first one, without having to worry about things like partial invalidation. > >>> +/* struct copy_propagation_state represents the accumulated knowledge >>> + * of the copy propagation pass while it scans through the code. Field >>> + * "variables" is a tree whose elements have type struct >>> + * copy_propagation_varible, and represent each of the variables the >>> + * pass has already encountered (except those with special semantics, >>> + * which are ignored). For each variable, the array "values" (whose >>> + * length is the register size of the variable) represent which node >>> + * and which index inside that node (i.e., which of the at most four >>> + * entries of a vector) provided that value last time. Field "node" >>> + * can be NULL, meaning that the pass was not able to statically >>> + * determine the node. >>> + */ >> >> This comment feels a bit too low-level? I dunno, comments like this >> are hard to review, but my inclination is to describe what you're >> doing at a high level, along the lines of "we track the last known >> value of each component of each variable", and let the code itself >> explain the details. > > Personally I would have loved to meet comments like mine in other areas > of Wine: knowing what data read or written by an algorithm are meant to > represent makes it much easier to understand why the algorithm does what > it does. I agree that this is not the most complicated case around, but > I thought this would be helpful for someone not already knowing my code. > OTOH I understand that also a high-level description is useful: would > you consider it satisfying to have both? Eh, maybe. I would at least start with the high-level explanation. When I read the above comment my eyes kind of glaze over, and it doesn't tell me anything that the code already doesn't (even the struct definitions alone tell me as much without checking how they're used.) I dunno, I'll see what you end up coming up with. BTW, I notice a spelling error ("copy_propagation_varible"). > >>> +static int copy_propagation_variable_compare(const void *key, const >>> struct rb_entry *entry) >>> +{ >>> + struct copy_propagation_variable *variable = >>> RB_ENTRY_VALUE(entry, struct copy_propagation_variable, entry); >>> + uintptr_t key_int = (uintptr_t)key, entry_int = >>> (uintptr_t)variable->var; >>> + >>> + if (key_int < entry_int) >>> + return -1; >>> + else if (key_int > entry_int) >>> + return 1; >>> + else >>> + return 0; >> >> Could we just modify the rbtree implementation to use uintptr_t >> instead, and then do a direct subtraction? > > I am not sure of what you mean: the pointer in struct > copy_propagation_variable is used as a pointer, it would not be very > logical to store it as an integer. > > Also, do you get the right think if you subtract two unsigned integers? > For one thing the result is unsigned, but even if you cast it to signed > you don't get an order, do you? > > Even if you could, you would get a shorter code, but I am not sure it > would be a more legible one. Er, sorry, I should have said intptr_t. I mean to return intptr_t from the rbtree compare callback, and then all you need is return (intptr_t)key - (intptr_t) variable->var; > >>> +static struct copy_propagation_variable >>> *copy_propagation_get_variable(struct hlsl_ctx *ctx, >>> + struct copy_propagation_state *state, struct hlsl_ir_var *var) >>> +{ >>> + struct rb_entry *entry = rb_get(&state->variables, var); >>> + struct copy_propagation_variable *variable; >>> + int res; >>> + >>> + if (entry) >>> + return RB_ENTRY_VALUE(entry, struct >>> copy_propagation_variable, entry); >>> + >>> + variable = hlsl_alloc(ctx, sizeof(*variable)); >>> + if (!variable) >>> + return NULL; >>> + >>> + variable->var = var; >>> + variable->values = hlsl_alloc(ctx, sizeof(*variable->values) * >>> var->data_type->reg_size); >>> + if (!variable->values) >>> + { >>> + vkd3d_free(variable); >>> + return NULL; >>> + } >>> + >>> + res = rb_put(&state->variables, var, &variable->entry); >>> + assert(!res); >> >> Although this is a bit awkward because most of the function is only >> relevant for stores, not loads, and I think it would actually be >> better to reflect that in the code, one way or another (so we can do >> less effort). > > Do less effort at what? It's true that when loading I could return NULL > when the variable has never been seen before (meaning that we don't know > anything about that variable, so the load cannot be removed) instead of > creating an "empty" variable placeholder and return that one, but I > don't really see an advantage doing that. It would seem more effort > rather than less to me, because there are more possible branches to check. Less effort done by the code, I mean, i.e. we use less memory that way and spend less time searching it. It requires a bit more code, but I think that if you slightly reorganize the callbacks it's still perfectly idiomatic. > > (BTW, that would also mean that the variable is read before being > written, which is something that shouldn't normally happen, but I don't > think it's sensible to count on that) I don't understand what you mean by this; can you please clarify? > >>> + if (TRACE_ON()) >>> + { >>> + char buf[32]; >>> + if (!node) >>> + sprintf(buf, "(nil)"); >> >> Can that happen? > > Not yet, but it will as soon as loops and conditionals enter the scene, > because they can cause the last store to a variable to become unknown > (at compilation time). Ah, of course, that makes sense. > > Now, I know that usually we don't like to have dead code around. I think > this case could be accepted, since this code is just a line long and > becomes alive at 3/5. Also, I find it easier to review this piece of > code at once, instead of first reviewing a part, checking that the > missing case will never happen and then review the missing case and > checking that it fits well in the already-reviewed part. > > But if you really don't like it, I can defer the NULL case to 3/5. Yeah, even here I find it easier to review if this code is moved to 3/5. > >>> + else if (node->index) >>> + sprintf(buf, "@%u", node->index); >>> + else >>> + sprintf(buf, "%p", node); >> >> Can that happen? > > Definitely. Arguably, it might be the common case, given that the > earlier optimization passes (including this pass itself, given that it > might be executed more than once) might synthesize a lot of nodes, which > won't have an index until compute_liveness is executed. > Ah. I assumed that you would actually be indexing right before copyprop, and didn't check whether that was actually the case. Although on reflection now I'm not sure if we *should* index. We only dump the IR once, at the end, and I feel like we risk confusion by printing instruction indices that won't match. Hmm, maybe the code makes sense as it is... >>> + TRACE("variable %s[%d] is written by %p[%d]\n", >>> variable->var->name, offset + index, buf, index); >> >> This trace doesn't really match the usual format, but more saliently, >> the hardcoded buffer is ugly. Assuming the other two cases really can >> happen, I'd rather see individual traces spelled out. >> >> Same thing below. > > Ok. As usual, I found find that uglier, but I can live with that. > > BTW, having the traces is not a condicio sine qua non for me. They were > useful for me to develop the patch, and I left them because I figured > that they might also be useful in the future, but we can happily get rid > of them. I'm typically in favor of leaving in traces if they were helpful during debugging. For what it's worth, I'd also be in favor of a "debugstr_node" helper or something. Maybe even if it uses a fixed-size buffer, although I'm not sure that Matteo or Henri will agree with that ;-) > >>> + struct hlsl_deref *src = &load->src; >>> + struct hlsl_ir_var *var = src->var; >>> + struct hlsl_ir_swizzle *swizzle; >>> + DWORD s; >>> + >>> + if (var->is_input_semantic || var->is_output_semantic || >>> var->is_uniform) >>> + return false; >> >> For input semantics and uniforms: yeah, but do we need to? The >> important question is "can we reconstruct a store for this variable", >> and this should already be false. >> >> For output semantics, we should never get here in the first place. > > I think you're right. I had put this check initially because I was > concentrated on other aspects of the algorithm, but it's true that it is > overly cautious. > >>> + >>> + if (type->type != HLSL_CLASS_SCALAR && type->type != >>> HLSL_CLASS_VECTOR) >>> + return false; >>> + >>> + offset = hlsl_offset_from_deref(src); >> >> The problem with hlsl_offset_from_deref(), and the reason I probably >> should have fought against 62b25bc52b in its current form, is that it >> not only doesn't deal with non-constant offsets, but doesn't really >> give the caller a way to bail either. We should probably be returning >> bool from it, and then aborting here. > > I agree. Actually, the first version of my patch set had a variant of > hlsl_offset_from_deref that acknowledged the possibility that it could > not be possible to statically determine the offset of a deref. This > condition is difficult to handle at code generation time, but it is not > particularly problematic here: at load time just ignore the load, and at > store time invalidate the whole variable. > > I can totally fix hlsl_offset_from_deref so that this happens. > > (speaking of constant expressions in the code, I am a bit bothered that > evaluate_array_dimension is not able to figure out the value of constant > expressions that are not immediate constants, and unfortunately fixing > that is not as easy as running a fold_constants pass; I don't in mind a > clean solution for that that doesn't require re-implementing constant > folding there; that's another matter, of course) Yeah, that one is worrying... I think we might have to reimplement constant folding. Probably we can at least use a common helper, though. > >>> + >>> + variable = copy_propagation_get_variable(ctx, state, var); >>> + if (!variable) >>> + return false; >>> + >>> + new_node = copy_propagation_reconstruct_node(variable, offset, >>> type->dimx, indices); >>> + >>> + if (TRACE_ON()) >>> + { >>> + char buf[32]; >>> + if (!new_node) >>> + sprintf(buf, "(nil)"); >> >> Is this useful to trace? > > As before, it is a matter of tastes, I believe. I can drop it, just as I > can drop the traces above. If it helps, feel free to leave it in. FWIW, I'm mostly talking about the "null" case, not the others. > >>> + else if (new_node->index) >>> + sprintf(buf, "@%u", new_node->index); >>> + else >>> + sprintf(buf, "%p", new_node); >> >> Can this happen? > > Yes, just as above. > >>> + variable = copy_propagation_get_variable(ctx, state, var); >>> + if (!variable) >>> + return false; >>> + >>> + copy_propagation_set_value(variable, >>> hlsl_offset_from_deref(lhs), store->writemask, store->rhs.node); >>> + >>> + return false; >>> +} >> >> Shouldn't this function just return void? > > It's a matter of philosophy. If you ask me, the switch in > copy_propagation_recursive should be rather oblivious of what the > various helpers actually do, it shouldn't have an insight that > copy_propagation_store never returns true. For its point of view, any > helper can potentially modify the code. Though I can change to void if > you prefer. It's a fair point, but I guess I look at copy_propagation_recursive() and think "recording stores should never make any progress; all the magic happens in rewriting loads". This becomes especially true if this function gets renamed to copy_prop_record_store() or something, which I think it should. > >>> +static bool copy_propagation_recursive(struct hlsl_ctx *ctx, struct >>> hlsl_block *block, >>> + struct copy_propagation_state *state) > > So, no comment for a function called "recursive" which is not (yet) > actually recursive? :-P I'll just go with "I requested the functions' names be changed anyway" ;-) I don't particularly mind this being called copy_prop_recurse(), even if it doesn't recurse yet, since I think it probably should (even if it's just as simple as invalidating the whole state on entry and exit of a CF block). On the other hand renaming to copy_prop_transform_block() or something would also be reasonable. > >>> @@ -1354,7 +1601,12 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, >>> struct hlsl_ir_function_decl *entry_fun >>> progress |= transform_ir(ctx, split_struct_copies, body, >>> NULL); >>> } >>> while (progress); >>> - while (transform_ir(ctx, fold_constants, body, NULL)); >>> + do >>> + { >>> + progress = transform_ir(ctx, fold_constants, body, NULL); >>> + progress |= copy_propagation_pass(ctx, body); >> >> This probably isn't worth examining, but I'm curious why constant >> folding is useful after copy-prop. > > Copy propagation might enable some additional steps of constant folding, > if a constant is first stored, than loaded and then an operation is > carried on it. OTOH, constant folding can enable some additional step of > copy propagation if it makes an offset statically visible. Ah, of course, that should have been obvious...

Nov. 10, 2021

Re: [PATCH 4/5] win32u: Move null user driver implementation from user32.

by Jacek Caban

On 11/10/21 3:19 PM, Rémi Bernon wrote: > On 11/10/21 15:01, Jacek Caban wrote: >> On 11/10/21 12:47 AM, Rémi Bernon wrote: >>> On 11/10/21 00:41, Rémi Bernon wrote: >>>> On 11/9/21 13:55, Jacek Caban wrote: >>>>> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> >>>>> --- >>>>> dlls/user32/driver.c | 209 +---------------------------- >>>>> dlls/win32u/driver.c | 310 >>>>> ++++++++++++++++++++++++++++++++++++++++++- >>>>> 2 files changed, 310 insertions(+), 209 deletions(-) >>>>> >>>>> >>>> >>>> A little bit related to my previous comment, this patch introduces >>>> a reset to "null" graphics driver on driver unload. >>>> >>>> I think it could also add the same >>>> >>>> __wine_set_display_driver( &null_driver, WINE_GDI_DRIVER_VERSION ); >>>> >>>> call when the "null" driver is explicitly requested and loaded, >>>> possibly addressing my previous comment too (as the pointer would >>>> not be lasy_load_driver anymore). >>> >>> Also, regarding EnumDisplayMonitors and GetMonitorInfo, for which >>> you added some FIXME, I have some patches to move the default >>> implementation back to the corresponding user32 functions. >>> >>> I intended to send them after the previous nulldrv display device >>> cache series I sent a week ago but I don't think they actually >>> depend on it. >>> >>> (Similarly, I used the monitor cache to keep the default and initial >>> monitor for GetMonitorInfo implementation) >>> >>> I rebased and pushed the patches there if you're interested, >>> although only the commit up to >>> c998fbadab6aa33e3bd2740b0651a0c0509d4c02 are really relevant for this: >> >> >> I have null driver functions and some other sysparams.c functions >> moved to win32u in my tree. Those needed to be changed due to to >> advapi32 and setupapi calls. I used a cache as well, but instead of >> having two caches, I extended adapters cache to contain missing >> monitor rects informations. There are a few functions that need to >> enumerate monitors, they would look like this: >> >> >> update_display_cache(); >> >> pthread_mutex_lock( &display_lock ); >> >> LIST_FOR_EACH_ENTRY( adapter, &adapters, struct display_device, >> entry ) >> { >> LIST_FOR_EACH_ENTRY( monitor, &adapter->children, struct >> monitor, dev.entry ) >> { >> >> /* .... */ >> >> } >> } >> >> pthread_mutex_unlock( &display_lock ); >> >> >> What do you think about such approach? >> > > Do you actually need to move the display device cache to win32u or is > it just because of the non-trivial nulldrv functions being moved there > too? It all needs to be moved. A number of other win32u functionality depends on those functions. > > In the tree I linked above, in the end after all the changes, the > nulldrv functions are all mostly no-op and the default implementation > lives in user32, updating the cache by enumerating setupapi devices. > > It also moves several things out of graphics drivers, such as reading > / writing current display mode, to the user32 EnumDisplaySettingsExW / > ChangeDisplaySettingsEx generic code. > > Overall I think it makes everything much simpler, and reduce the > amount of entry points graphics driver would need, and reduce the > amount of changes needed if you need to replace all setupapi / > advapi32 calls. Yes, I think that those are mostly the right direction. I could even imagine setting registry in wineandroid.drv and remove pEnumDisplayMonitors entry point. I already replaced problematic setupapi / advapi32 calls from sysparams.c in my tree. I'd prefer cache layout like I mentioned, but it should be easy to adopt to a different cache layout as well. Thanks, Jacek

Nov. 10, 2021

Re: [PATCH vkd3d 5/5] vkd3d-shader/hlsl: Write SM4 sample instructions.

by Zebediah Figura (she/her)

On 11/10/21 04:30, Henri Verbeet wrote: > On Tue, 9 Nov 2021 at 04:56, Zebediah Figura <zfigura(a)codeweavers.com> wrote: >> case HLSL_RESOURCE_SAMPLE: >> - hlsl_fixme(ctx, load->node.loc, "Resource sample instruction."); >> + if (!load->sampler.var) >> + hlsl_fixme(ctx, load->node.loc, "SM4 combined sample expression.\n"); > > Is the extra newline intentional here? > Nope, that was a mistake.

Nov. 10, 2021

Re: [PATCH v5 2/2] mshtml: Populate the element props properly.

by Gabriel Ivăncescu

On 10/11/2021 17:29, Jacek Caban wrote: > Hi Gabriel, > > On 11/9/21 10:00 PM, Gabriel Ivăncescu wrote: >> --- a/dlls/mshtml/htmlelem.c >> +++ b/dlls/mshtml/htmlelem.c >> @@ -6516,7 +6516,10 @@ static HRESULT >> HTMLElement_populate_props(DispatchEx *dispex) >> } else >> V_BSTR(&value) = NULL; >> - IHTMLElement_setAttribute(&This->IHTMLElement_iface, name, >> value, 0); >> + hres = IDispatchEx_GetDispID(&dispex->IDispatchEx_iface, >> name, fdexNameEnsure | fdexNameCaseInsensitive, &id); >> + if(SUCCEEDED(hres)) >> + set_elem_attr_value_by_dispid(This, id, &value); > > > While this is probably the right thing for compat modes <IE9, later > modes should not really need it. They should not expose attributes as JS > properties, see the attached test. I think that entire > HTMLElement_populate_props should be no-op on IE9+. That means that > current attributes collection will not work for those cases, but AFICS > it's already broken. The right fix for attributes collection would be to > have its variant based on something like nsIDOMMozNamedAttrMap. > > > Thanks, > > Jacek > Ah, thanks for noticing, I'll just exit early on IE9+ then. This gives me more reasons to attempt to fix it at some point since already had the issue with toString exposed as an attribute...

Nov. 10, 2021

[PATCH 10/10] dbghelp/dwarf: workaround functions with multiple range of addresses

by Eric Pouech

gcc can emit functions with code spread across non contiguous code areas We used to register those functions with an address range enclosing all ranges (meaning that all addresses not actually belonging to the function but lying in that address range could be returned by dbghelp as belonging to the function) Work around this by registering the function with only the first range of addresses (this will avoid the errors described above), but will fail to mark the other address ranges as part of the function. dbghelp doesn't seem to have explicit support of those cases, even if pdb/codeview also support functions with multi range of addresses (see S_SEPCODE) Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/dwarf.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dlls/dbghelp/dwarf.c b/dlls/dbghelp/dwarf.c index 573c56627a7..ef17f7d539f 100644 --- a/dlls/dbghelp/dwarf.c +++ b/dlls/dbghelp/dwarf.c @@ -2239,7 +2239,8 @@ static void dwarf2_parse_subprogram_block(dwarf2_subprogram_t* subpgm, static struct symt* dwarf2_parse_subprogram(dwarf2_debug_info_t* di) { struct attribute name; - ULONG_PTR low_pc, high_pc; + struct addr_range* addr_ranges; + unsigned num_addr_ranges; struct attribute is_decl; struct attribute inline_flags; struct symt* ret_type; @@ -2276,7 +2277,7 @@ static struct symt* dwarf2_parse_subprogram(dwarf2_debug_info_t* di) /* it's a real declaration, skip it */ return NULL; } - if (!dwarf2_read_range(di->unit_ctx, di, &low_pc, &high_pc)) + if ((addr_ranges = dwarf2_get_ranges(di, &num_addr_ranges)) == NULL) { WARN("cannot get range for %s\n", debugstr_a(name.u.string)); return NULL; @@ -2285,7 +2286,7 @@ static struct symt* dwarf2_parse_subprogram(dwarf2_debug_info_t* di) * (not the case for stabs), we just drop Wine's thunks here... * Actual thunks will be created in elf_module from the symbol table */ - if (elf_is_in_thunk_area(di->unit_ctx->module_ctx->load_offset + low_pc, di->unit_ctx->module_ctx->thunks) >= 0) + if (elf_is_in_thunk_area(di->unit_ctx->module_ctx->load_offset + addr_ranges[0].low, di->unit_ctx->module_ctx->thunks) >= 0) return NULL; ret_type = dwarf2_lookup_type(di); @@ -2293,8 +2294,11 @@ static struct symt* dwarf2_parse_subprogram(dwarf2_debug_info_t* di) sig_type = symt_new_function_signature(di->unit_ctx->module_ctx->module, ret_type, CV_CALL_FAR_C); subpgm.top_func = symt_new_function(di->unit_ctx->module_ctx->module, di->unit_ctx->compiland, dwarf2_get_cpp_name(di, name.u.string), - di->unit_ctx->module_ctx->load_offset + low_pc, high_pc - low_pc, - &sig_type->symt); + addr_ranges[0].low, addr_ranges[0].high - addr_ranges[0].low, &sig_type->symt); + if (num_addr_ranges > 1) + WARN("Function %s has multiple address ranges, only using the first one\n", name.u.string); + free(addr_ranges); + subpgm.current_func = subpgm.top_func; di->symt = &subpgm.top_func->symt; subpgm.ctx = di->unit_ctx;

Nov. 10, 2021

[PATCH 09/10] dbghelp/dwarf: introduce a helper to read properly multiple range of addresses

by Eric Pouech

use it to reimplement dwarf2_feed_inlined_ranges Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/dbghelp/dwarf.c | 61 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/dlls/dbghelp/dwarf.c b/dlls/dbghelp/dwarf.c index 90452d74eaf..573c56627a7 100644 --- a/dlls/dbghelp/dwarf.c +++ b/dlls/dbghelp/dwarf.c @@ -1272,32 +1272,40 @@ static BOOL dwarf2_read_range(dwarf2_parse_context_t* ctx, const dwarf2_debug_in } } -static BOOL dwarf2_feed_inlined_ranges(dwarf2_parse_context_t* ctx, const dwarf2_debug_info_t* di, - struct symt_inlinesite* inlined) +static struct addr_range* dwarf2_get_ranges(const dwarf2_debug_info_t* di, unsigned* num_ranges) { struct attribute range; + struct addr_range* ranges; if (dwarf2_find_attribute(di, DW_AT_ranges, &range)) { dwarf2_traverse_context_t traverse; + unsigned alloc = 16; + ranges = malloc(sizeof(struct addr_range) * alloc); + if (!ranges) return NULL; + *num_ranges = 0; - traverse.data = ctx->module_ctx->sections[section_ranges].address + range.u.uvalue; - traverse.end_data = ctx->module_ctx->sections[section_ranges].address + - ctx->module_ctx->sections[section_ranges].size; + traverse.data = di->unit_ctx->module_ctx->sections[section_ranges].address + range.u.uvalue; + traverse.end_data = di->unit_ctx->module_ctx->sections[section_ranges].address + + di->unit_ctx->module_ctx->sections[section_ranges].size; - while (traverse.data + 2 * ctx->head.word_size < traverse.end_data) + while (traverse.data + 2 * di->unit_ctx->head.word_size < traverse.end_data) { - ULONG_PTR low = dwarf2_parse_addr_head(&traverse, &ctx->head); - ULONG_PTR high = dwarf2_parse_addr_head(&traverse, &ctx->head); + ULONG_PTR low = dwarf2_parse_addr_head(&traverse, &di->unit_ctx->head); + ULONG_PTR high = dwarf2_parse_addr_head(&traverse, &di->unit_ctx->head); if (low == 0 && high == 0) break; - if (low == (ctx->head.word_size == 8 ? (~(DWORD64)0u) : (DWORD64)(~0u))) + if (low == (di->unit_ctx->head.word_size == 8 ? (~(DWORD64)0u) : (DWORD64)(~0u))) FIXME("unsupported yet (base address selection)\n"); - /* range values are relative to start of compilation unit */ - symt_add_inlinesite_range(ctx->module_ctx->module, inlined, - ctx->compiland->address + low, ctx->compiland->address + high); + if (*num_ranges >= alloc) + { + alloc *= 2; + ranges = realloc(ranges, sizeof(struct addr_range) * alloc); + if (!ranges) return NULL; + } + ranges[*num_ranges].low = di->unit_ctx->compiland->address + low; + ranges[*num_ranges].high = di->unit_ctx->compiland->address + high; + (*num_ranges)++; } - - return TRUE; } else { @@ -1306,8 +1314,8 @@ static BOOL dwarf2_feed_inlined_ranges(dwarf2_parse_context_t* ctx, const dwarf2 if (!dwarf2_find_attribute(di, DW_AT_low_pc, &low_pc) || !dwarf2_find_attribute(di, DW_AT_high_pc, &high_pc)) - return FALSE; - if (ctx->head.version >= 4) + return NULL; + if (di->unit_ctx->head.version >= 4) switch (high_pc.form) { case DW_FORM_addr: @@ -1325,11 +1333,13 @@ static BOOL dwarf2_feed_inlined_ranges(dwarf2_parse_context_t* ctx, const dwarf2 FIXME("Unsupported class for high_pc\n"); break; } - symt_add_inlinesite_range(ctx->module_ctx->module, inlined, - ctx->module_ctx->load_offset + low_pc.u.uvalue, - ctx->module_ctx->load_offset + high_pc.u.uvalue); - return TRUE; + ranges = malloc(sizeof(struct addr_range)); + if (!ranges) return NULL; + ranges[0].low = di->unit_ctx->module_ctx->load_offset + low_pc.u.uvalue; + ranges[0].high = di->unit_ctx->module_ctx->load_offset + high_pc.u.uvalue; + *num_ranges = 1; } + return ranges; } /****************************************************************** @@ -2072,6 +2082,8 @@ static void dwarf2_parse_inlined_subroutine(dwarf2_subprogram_t* subpgm, struct vector* children; dwarf2_debug_info_t*child; unsigned int i; + struct addr_range* adranges; + unsigned num_adranges; TRACE("%s\n", dwarf2_debug_di(di)); @@ -2099,7 +2111,14 @@ static void dwarf2_parse_inlined_subroutine(dwarf2_subprogram_t* subpgm, subpgm->current_func = (struct symt_function*)inlined; subpgm->current_block = NULL; - if (!dwarf2_feed_inlined_ranges(subpgm->ctx, di, inlined)) + if ((adranges = dwarf2_get_ranges(di, &num_adranges)) != NULL) + { + for (i = 0; i < num_adranges; ++i) + symt_add_inlinesite_range(subpgm->ctx->module_ctx->module, inlined, + adranges[i].low, adranges[i].high); + free(adranges); + } + else WARN("cannot read ranges\n"); children = dwarf2_get_di_children(di);

Nov. 10, 2021

[PATCH 08/10] tools/winedump: better handling and display of nested symbol entries

by Eric Pouech

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- tools/winedump/msc.c | 142 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 101 insertions(+), 41 deletions(-) diff --git a/tools/winedump/msc.c b/tools/winedump/msc.c index 07691e5be6a..b566018ac89 100644 --- a/tools/winedump/msc.c +++ b/tools/winedump/msc.c @@ -1303,11 +1303,61 @@ static void dump_binannot(const unsigned char* ba, const char* last, unsigned in } } +struct symbol_dumper +{ + unsigned depth; + unsigned alloc; + struct + { + unsigned end; + const union codeview_symbol* sym; + }* stack; +}; + +static void init_symbol_dumper(struct symbol_dumper* sd) +{ + sd->depth = 0; + sd->alloc = 16; + sd->stack = malloc(sd->alloc * sizeof(sd->stack[0])); +} + +static void push_symbol_dumper(struct symbol_dumper* sd, const union codeview_symbol* sym, unsigned end) +{ + if (!sd->stack) return; + if (sd->depth >= sd->alloc && + !(sd->stack = realloc(sd->stack, (sd->alloc *= 2) * sizeof(sd->stack[0])))) + return; + sd->stack[sd->depth].end = end; + sd->stack[sd->depth].sym = sym; + sd->depth++; +} + +static unsigned short pop_symbol_dumper(struct symbol_dumper* sd, unsigned end) +{ + if (!sd->stack) return 0; + if (!sd->depth) + { + printf(">>> Error in stack\n"); + return 0; + } + sd->depth--; + if (sd->stack[sd->depth].end != end) + printf(">>> Wrong end reference\n"); + return sd->stack[sd->depth].sym->generic.id; +} + +static void dispose_symbol_dumper(struct symbol_dumper* sd) +{ + free(sd->stack); +} + BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long size) { unsigned int i; int length; - int nest_block = 0; + struct symbol_dumper sd; + + init_symbol_dumper(&sd); /* * Loop over the different types of records and whenever we * find something we are interested in, record it and move on. @@ -1315,12 +1365,23 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long for (i = start; i < size; i += length) { const union codeview_symbol* sym = (const union codeview_symbol*)((const char*)root + i); - unsigned indent; + unsigned indent, ref; length = sym->generic.len + 2; if (!sym->generic.id || length < 4) break; indent = printf(" %04x => ", i); + switch (sym->generic.id) + { + case S_END: + case S_INLINESITE_END: + indent += printf("%*s", 2 * sd.depth - 2, ""); + break; + default: + indent += printf("%*s", 2 * sd.depth, ""); + break; + } + switch (sym->generic.id) { /* @@ -1386,6 +1447,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long p_string(&sym->thunk_v1.p_name), sym->thunk_v1.segment, sym->thunk_v1.offset, sym->thunk_v1.thunk_len, sym->thunk_v1.thtype); + push_symbol_dumper(&sd, sym, sym->thunk_v1.pend); break; case S_THUNK32: @@ -1393,47 +1455,38 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long sym->thunk_v3.name, sym->thunk_v3.segment, sym->thunk_v3.offset, sym->thunk_v3.thunk_len, sym->thunk_v3.thtype); + push_symbol_dumper(&sd, sym, sym->thunk_v3.pend); break; /* Global and static functions */ case S_GPROC32_16t: case S_LPROC32_16t: printf("%s-Proc V1: '%s' (%04x:%08x#%x) type:%x attr:%x\n", - sym->generic.id == S_GPROC32_16t ? "Global" : "-Local", + sym->generic.id == S_GPROC32_16t ? "Global" : "Local", p_string(&sym->proc_v1.p_name), sym->proc_v1.segment, sym->proc_v1.offset, sym->proc_v1.proc_len, sym->proc_v1.proctype, sym->proc_v1.flags); printf("%*s\\- Debug: start=%08x end=%08x\n", indent, "", sym->proc_v1.debug_start, sym->proc_v1.debug_end); - if (nest_block) - { - printf(">>> prev func still has nest_block %u count\n", nest_block); - nest_block = 0; - } -/* EPP unsigned int pparent; */ -/* EPP unsigned int pend; */ -/* EPP unsigned int next; */ + printf("%*s\\- parent:<%x> end:<%x> next<%x>\n", + indent, "", sym->proc_v1.pparent, sym->proc_v1.pend, sym->proc_v1.next); + push_symbol_dumper(&sd, sym, sym->proc_v1.pend); break; case S_GPROC32_ST: case S_LPROC32_ST: printf("%s-Proc V2: '%s' (%04x:%08x#%x) type:%x attr:%x\n", - sym->generic.id == S_GPROC32_ST ? "Global" : "-Local", + sym->generic.id == S_GPROC32_ST ? "Global" : "Local", p_string(&sym->proc_v2.p_name), sym->proc_v2.segment, sym->proc_v2.offset, sym->proc_v2.proc_len, sym->proc_v2.proctype, sym->proc_v2.flags); printf("%*s\\- Debug: start=%08x end=%08x\n", indent, "", sym->proc_v2.debug_start, sym->proc_v2.debug_end); - if (nest_block) - { - printf(">>> prev func still has nest_block %u count\n", nest_block); - nest_block = 0; - } -/* EPP unsigned int pparent; */ -/* EPP unsigned int pend; */ -/* EPP unsigned int next; */ + printf("%*s\\- parent:<%x> end:<%x> next<%x>\n", + indent, "", sym->proc_v2.pparent, sym->proc_v2.pend, sym->proc_v2.next); + push_symbol_dumper(&sd, sym, sym->proc_v2.pend); break; case S_LPROC32: @@ -1446,14 +1499,9 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long sym->proc_v3.flags); printf("%*s\\- Debug: start=%08x end=%08x\n", indent, "", sym->proc_v3.debug_start, sym->proc_v3.debug_end); - if (nest_block) - { - printf(">>> prev func still has nest_block %u count\n", nest_block); - nest_block = 0; - } -/* EPP unsigned int pparent; */ -/* EPP unsigned int pend; */ -/* EPP unsigned int next; */ + printf("%*s\\- parent:<%x> end:<%x> next<%x>\n", + indent, "", sym->proc_v3.pparent, sym->proc_v3.pend, sym->proc_v3.next); + push_symbol_dumper(&sd, sym, sym->proc_v3.pend); break; /* Function parameters and stack variables */ @@ -1503,15 +1551,15 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long p_string(&sym->block_v1.p_name), sym->block_v1.segment, sym->block_v1.offset, sym->block_v1.length); - nest_block++; + push_symbol_dumper(&sd, sym, sym->block_v1.end); break; case S_BLOCK32: - printf("Block V3 '%s' (%04x:%08x#%08x) parent:%u end:%x\n", + printf("Block V3 '%s' (%04x:%08x#%08x) parent:<%u> end:<%x>\n", sym->block_v3.name, sym->block_v3.segment, sym->block_v3.offset, sym->block_v3.length, sym->block_v3.parent, sym->block_v3.end); - nest_block++; + push_symbol_dumper(&sd, sym, sym->block_v3.end); break; /* Additional function information */ @@ -1532,13 +1580,17 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long break; case S_END: - if (nest_block) + ref = sd.depth ? (const char*)sd.stack[sd.depth - 1].sym - (const char*)root : 0; + switch (pop_symbol_dumper(&sd, i)) { - nest_block--; - printf("End-Of block (%u)\n", nest_block); + case S_BLOCK32_ST: + case S_BLOCK32: + printf("End-Of block <%x>\n", ref); + break; + default: + printf("End-Of <%x>\n", ref); + break; } - else - printf("End-Of function\n"); break; case S_COMPILE: @@ -1813,18 +1865,24 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long break; case S_INLINESITE: - printf("Inline-site V3 parent:%x end:%x inlinee:%x\n", + printf("Inline-site V3 parent:<%x> end:<%x> inlinee:%x\n", sym->inline_site_v3.pParent, sym->inline_site_v3.pEnd, sym->inline_site_v3.inlinee); dump_binannot(sym->inline_site_v3.binaryAnnotations, get_last(sym), indent); + push_symbol_dumper(&sd, sym, sym->inline_site_v3.pEnd); break; + case S_INLINESITE2: - printf("Inline-site2 V3 parent:%x end:%x inlinee:%x #inv:%u\n", + printf("Inline-site2 V3 parent:<%x> end:<%x> inlinee:%x #inv:%u\n", sym->inline_site2_v3.pParent, sym->inline_site2_v3.pEnd, sym->inline_site2_v3.inlinee, sym->inline_site2_v3.invocations); dump_binannot(sym->inline_site2_v3.binaryAnnotations, get_last(sym), indent); + push_symbol_dumper(&sd, sym, sym->inline_site2_v3.pEnd); break; + case S_INLINESITE_END: - printf("Inline-site-end\n"); + ref = sd.depth ? (const char*)sd.stack[sd.depth - 1].sym - (const char*)root : 0; + pop_symbol_dumper(&sd, i); + printf("Inline-site-end <%x>\n", ref); break; case S_CALLEES: @@ -1843,7 +1901,8 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long ninvoc = (const unsigned*)get_last(sym) - invoc; for (i = 0; i < sym->function_list_v3.count; ++i) - printf("%*s| func:%x invoc:%u\n", indent, "", sym->function_list_v3.funcs[i], i < ninvoc ? invoc[i] : 0); + printf("%*s| func:%x invoc:%u\n", + indent, "", sym->function_list_v3.funcs[i], i < ninvoc ? invoc[i] : 0); } break; @@ -1872,7 +1931,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long break; case S_SEPCODE: - printf("SepCode V3 pParent:%x pEnd:%x separated:%04x:%08x (#%u) from %04x:%08x\n", + printf("SepCode V3 parent:<%x> end:<%x> separated:%04x:%08x (#%u) from %04x:%08x\n", sym->sepcode_v3.pParent, sym->sepcode_v3.pEnd, sym->sepcode_v3.sect, sym->sepcode_v3.off, sym->sepcode_v3.length, sym->sepcode_v3.sectParent, sym->sepcode_v3.offParent); @@ -1894,6 +1953,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long dump_data((const void*)sym, sym->generic.len + 2, " "); } } + dispose_symbol_dumper(&sd); return TRUE; }

Nov. 10, 2021

[PATCH 07/10] tools/winedump/msc: properly indent multi lines symbol records

by Eric Pouech

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- tools/winedump/msc.c | 84 ++++++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/tools/winedump/msc.c b/tools/winedump/msc.c index 2fefa36381a..07691e5be6a 100644 --- a/tools/winedump/msc.c +++ b/tools/winedump/msc.c @@ -1196,13 +1196,13 @@ BOOL codeview_dump_types_from_block(const void* table, unsigned long len) return TRUE; } -static void dump_defrange(const struct cv_addr_range* range, const void* last, const char* pfx) +static void dump_defrange(const struct cv_addr_range* range, const void* last, unsigned indent) { const struct cv_addr_gap* gap; - printf("%s%04x:%08x range:#%x\n", pfx, range->isectStart, range->offStart, range->cbRange); + printf("%*s\\- %04x:%08x range:#%x\n", indent, "", range->isectStart, range->offStart, range->cbRange); for (gap = (const struct cv_addr_gap*)(range + 1); (const void*)(gap + 1) <= last; ++gap) - printf("%s\toffset:%x range:#%x\n", pfx, gap->gapStartOffset, gap->cbRange); + printf("%*s | offset:%x range:#%x\n", indent, "", gap->gapStartOffset, gap->cbRange); } /* return address of first byte after the symbol */ @@ -1240,7 +1240,7 @@ static inline int binannot_getsigned(unsigned i) return (i & 1) ? -(int)(i >> 1) : (i >> 1); } -static void dump_binannot(const unsigned char* ba, const char* last, const char* pfx) +static void dump_binannot(const unsigned char* ba, const char* last, unsigned indent) { while (ba < (const unsigned char*)last) { @@ -1249,56 +1249,56 @@ static void dump_binannot(const unsigned char* ba, const char* last, const char* { case BA_OP_Invalid: /* not clear if param? */ - printf("%sInvalid\n", pfx); + printf("%*s | Invalid\n", indent, ""); break; case BA_OP_CodeOffset: - printf("%sCodeOffset %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | CodeOffset %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeCodeOffsetBase: - printf("%sChangeCodeOffsetBase %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeCodeOffsetBase %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeCodeOffset: - printf("%sChangeCodeOffset %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeCodeOffset %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeCodeLength: - printf("%sChangeCodeLength %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeCodeLength %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeFile: - printf("%sChangeFile %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeFile %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeLineOffset: - printf("%sChangeLineOffset %d\n", pfx, binannot_getsigned(binannot_uncompress(&ba))); + printf("%*s | ChangeLineOffset %d\n", indent, "", binannot_getsigned(binannot_uncompress(&ba))); break; case BA_OP_ChangeLineEndDelta: - printf("%sChangeLineEndDelta %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeLineEndDelta %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeRangeKind: - printf("%sChangeRangeKind %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeRangeKind %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeColumnStart: - printf("%sChangeColumnStart %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeColumnStart %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeColumnEndDelta: - printf("%sChangeColumnEndDelta %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeColumnEndDelta %u\n", indent, "", binannot_uncompress(&ba)); break; case BA_OP_ChangeCodeOffsetAndLineOffset: { unsigned p1 = binannot_uncompress(&ba); - printf("%sChangeCodeOffsetAndLineOffset %u %u (0x%x)\n", pfx, p1 & 0xf, binannot_getsigned(p1 >> 4), p1); + printf("%*s | ChangeCodeOffsetAndLineOffset %u %u (0x%x)\n", indent, "", p1 & 0xf, binannot_getsigned(p1 >> 4), p1); } break; case BA_OP_ChangeCodeLengthAndCodeOffset: { unsigned p1 = binannot_uncompress(&ba); unsigned p2 = binannot_uncompress(&ba); - printf("%sChangeCodeLengthAndCodeOffset %u %u\n", pfx, p1, p2); + printf("%*s | ChangeCodeLengthAndCodeOffset %u %u\n", indent, "", p1, p2); } break; case BA_OP_ChangeColumnEnd: - printf("%sChangeColumnEnd %u\n", pfx, binannot_uncompress(&ba)); + printf("%*s | ChangeColumnEnd %u\n", indent, "", binannot_uncompress(&ba)); break; - default: printf("%sUnsupported op %d %x\n", pfx, opcode, opcode); /* may cause issues because of param */ + default: printf(">>> Unsupported op %d %x\n", opcode, opcode); /* may cause issues because of param */ } } } @@ -1315,9 +1315,11 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long for (i = start; i < size; i += length) { const union codeview_symbol* sym = (const union codeview_symbol*)((const char*)root + i); + unsigned indent; + length = sym->generic.len + 2; if (!sym->generic.id || length < 4) break; - printf("\t%04x => ", i); + indent = printf(" %04x => ", i); switch (sym->generic.id) { @@ -1402,8 +1404,8 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long sym->proc_v1.segment, sym->proc_v1.offset, sym->proc_v1.proc_len, sym->proc_v1.proctype, sym->proc_v1.flags); - printf("\t\tDebug: start=%08x end=%08x\n", - sym->proc_v1.debug_start, sym->proc_v1.debug_end); + printf("%*s\\- Debug: start=%08x end=%08x\n", + indent, "", sym->proc_v1.debug_start, sym->proc_v1.debug_end); if (nest_block) { printf(">>> prev func still has nest_block %u count\n", nest_block); @@ -1422,8 +1424,8 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long sym->proc_v2.segment, sym->proc_v2.offset, sym->proc_v2.proc_len, sym->proc_v2.proctype, sym->proc_v2.flags); - printf("\t\tDebug: start=%08x end=%08x\n", - sym->proc_v2.debug_start, sym->proc_v2.debug_end); + printf("%*s\\- Debug: start=%08x end=%08x\n", + indent, "", sym->proc_v2.debug_start, sym->proc_v2.debug_end); if (nest_block) { printf(">>> prev func still has nest_block %u count\n", nest_block); @@ -1442,8 +1444,8 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long sym->proc_v3.segment, sym->proc_v3.offset, sym->proc_v3.proc_len, sym->proc_v3.proctype, sym->proc_v3.flags); - printf("\t\tDebug: start=%08x end=%08x\n", - sym->proc_v3.debug_start, sym->proc_v3.debug_end); + printf("%*s\\- Debug: start=%08x end=%08x\n", + indent, "", sym->proc_v3.debug_start, sym->proc_v3.debug_end); if (nest_block) { printf(">>> prev func still has nest_block %u count\n", nest_block); @@ -1559,7 +1561,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long const char* ptr = sym->compile2_v2.p_name.name + sym->compile2_v2.p_name.namelen; while (*ptr) { - printf("\t\t%s => ", ptr); ptr += strlen(ptr) + 1; + printf("%*s| %s => ", indent, "", ptr); ptr += strlen(ptr) + 1; printf("%s\n", ptr); ptr += strlen(ptr) + 1; } } @@ -1577,7 +1579,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long const char* ptr = sym->compile2_v3.name + strlen(sym->compile2_v3.name) + 1; while (*ptr) { - printf("\t\t%s => ", ptr); ptr += strlen(ptr) + 1; + printf("%*s| %s => ", indent, "", ptr); ptr += strlen(ptr) + 1; printf("%s\n", ptr); ptr += strlen(ptr) + 1; } } @@ -1598,12 +1600,12 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long const char* x1 = (const char*)sym + 4 + 1; const char* x2; - printf("\tTool conf V3\n"); + printf("Tool conf V3\n"); while (*x1) { x2 = x1 + strlen(x1) + 1; if (!*x2) break; - printf("\t\t%s: %s\n", x1, x2); + printf("%*s| %s: %s\n", indent, "", x1, x2); x1 = x2 + strlen(x2) + 1; } } @@ -1696,7 +1698,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long pname = PSTRING(sym, length); length += (pname->namelen + 1 + 3) & ~3; - printf("\t%08x %08x %08x '%s'\n", + printf("%08x %08x %08x '%s'\n", *(((const DWORD*)sym) + 1), *(((const DWORD*)sym) + 2), *(((const DWORD*)sym) + 3), p_string(pname)); } @@ -1766,22 +1768,22 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long case S_DEFRANGE: printf("DefRange dia:%x\n", sym->defrange_v3.program); - dump_defrange(&sym->defrange_v3.range, get_last(sym), "\t\t"); + dump_defrange(&sym->defrange_v3.range, get_last(sym), indent); break; case S_DEFRANGE_SUBFIELD: printf("DefRange-subfield V3 dia:%x off-parent:%x\n", sym->defrange_subfield_v3.program, sym->defrange_subfield_v3.offParent); - dump_defrange(&sym->defrange_subfield_v3.range, get_last(sym), "\t\t"); + dump_defrange(&sym->defrange_subfield_v3.range, get_last(sym), indent); break; case S_DEFRANGE_REGISTER: printf("DefRange-register V3 reg:%x attr-unk:%x\n", sym->defrange_register_v3.reg, sym->defrange_register_v3.attr); - dump_defrange(&sym->defrange_register_v3.range, get_last(sym), "\t\t"); + dump_defrange(&sym->defrange_register_v3.range, get_last(sym), indent); break; case S_DEFRANGE_FRAMEPOINTER_REL: printf("DefRange-framepointer-rel V3 offFP:%x\n", sym->defrange_frameptrrel_v3.offFramePointer); - dump_defrange(&sym->defrange_frameptrrel_v3.range, get_last(sym), "\t\t"); + dump_defrange(&sym->defrange_frameptrrel_v3.range, get_last(sym), indent); break; case S_DEFRANGE_FRAMEPOINTER_REL_FULL_SCOPE: printf("DefRange-framepointer-rel-fullscope V3 offFP:%x\n", @@ -1792,13 +1794,13 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long sym->defrange_subfield_register_v3.reg, sym->defrange_subfield_register_v3.attr, sym->defrange_subfield_register_v3.offParent); - dump_defrange(&sym->defrange_subfield_register_v3.range, get_last(sym), "\t\t"); + dump_defrange(&sym->defrange_subfield_register_v3.range, get_last(sym), indent); break; case S_DEFRANGE_REGISTER_REL: printf("DefRange-register-rel V3 reg:%x off-parent:%x off-BP:%x\n", sym->defrange_registerrel_v3.baseReg, sym->defrange_registerrel_v3.offsetParent, sym->defrange_registerrel_v3.offBasePointer); - dump_defrange(&sym->defrange_registerrel_v3.range, get_last(sym), "\t\t"); + dump_defrange(&sym->defrange_registerrel_v3.range, get_last(sym), indent); break; case S_CALLSITEINFO: @@ -1813,13 +1815,13 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long case S_INLINESITE: printf("Inline-site V3 parent:%x end:%x inlinee:%x\n", sym->inline_site_v3.pParent, sym->inline_site_v3.pEnd, sym->inline_site_v3.inlinee); - dump_binannot(sym->inline_site_v3.binaryAnnotations, get_last(sym), "\t\t"); + dump_binannot(sym->inline_site_v3.binaryAnnotations, get_last(sym), indent); break; case S_INLINESITE2: printf("Inline-site2 V3 parent:%x end:%x inlinee:%x #inv:%u\n", sym->inline_site2_v3.pParent, sym->inline_site2_v3.pEnd, sym->inline_site2_v3.inlinee, sym->inline_site2_v3.invocations); - dump_binannot(sym->inline_site2_v3.binaryAnnotations, get_last(sym), "\t\t"); + dump_binannot(sym->inline_site2_v3.binaryAnnotations, get_last(sym), indent); break; case S_INLINESITE_END: printf("Inline-site-end\n"); @@ -1841,7 +1843,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long ninvoc = (const unsigned*)get_last(sym) - invoc; for (i = 0; i < sym->function_list_v3.count; ++i) - printf("\t\tfunc:%x invoc:%u\n", sym->function_list_v3.funcs[i], i < ninvoc ? invoc[i] : 0); + printf("%*s| func:%x invoc:%u\n", indent, "", sym->function_list_v3.funcs[i], i < ninvoc ? invoc[i] : 0); } break; @@ -1883,7 +1885,7 @@ BOOL codeview_dump_symbols(const void* root, unsigned long start, unsigned long const char* ptr = sym->annotation_v3.rgsz; const char* last = ptr + sym->annotation_v3.csz; for (; ptr < last; ptr += strlen(ptr) + 1) - printf("\t%s\n", ptr); + printf("%*s| %s\n", indent, "", ptr); } break;

Nov. 10, 2021