This is all working now, including using std::aligned_storage to avoid undefined behavior. You can see the ready-for-review CL at crrev.com/c/3042215. It drops the number of V8 translation units that include windows.h in a build of Chrome for Windows from 760+ to 16. Not bad.
On Fri, Jul 23, 2021 at 8:36 AM Bruce Dawson <[email protected]> wrote: > Okay, that makes sense. In Chromium you just need a blank line but I'll > add an explanatory comment as well. > > Any thoughts on intrin.h? It works in my debug builds in the context of > Chromium, but not on the bots. I think intrin.h must be different there. I > can keep exploring if nobody knows off the top of their head. > > On Fri, Jul 23, 2021 at 8:34 AM Dan Elphick <[email protected]> wrote: > >> git cl format won't reorder headers separated by a comment (possibly >> needs a newline as well but that looks more natural anyway). >> >> On Fri, 23 Jul 2021 at 16:27, 'Bruce Dawson' via v8-dev < >> [email protected]> wrote: >> >>> I'll try std::aligned_storage. It sounds like that will be perfect. I >>> can just drop it in to replace the existing declarations that I created. >>> >>> You can see the 99% working CL at crrev.com/c//3042215. The only >>> problem is that something doesn't work with intrin.h on cppgc. I include >>> intrin.h and then get an error saying: >>> >>> note: include the header <intrin.h> or explicitly provide a declaration >>> for '__readfsdword' >>> >>> >>> I'm also hitting problems where "git cl format" reorders my #includes in >>> a way that breaks compilation and I don't know how to prevent that. If I >>> can get suggestions for those two issues then I can finish this off. >>> >>> On Fri, Jul 23, 2021 at 7:44 AM Leszek Swirski <[email protected]> >>> wrote: >>> >>>> Or by using std::aligned_storage >>>> <https://en.cppreference.com/w/cpp/types/aligned_storage> + some >>>> static asserts that sizes match, if we need to actually allocate storage >>>> for these types... >>>> >>>> On Fri, Jul 23, 2021 at 4:40 PM Leszek Swirski <[email protected]> >>>> wrote: >>>> >>>>> I don't suppose we could just type erase via void*? That should at >>>>> least be UB-clean. I do think we, in reality, have also given up on strict >>>>> aliasing, but we nevertheless do try to go via bitcast where possible. I >>>>> kicked off some UBSan bots on your CL, let's see what they say. >>>>> >>>>> On Thu, Jul 22, 2021 at 9:43 PM 'Bruce Dawson' via v8-dev < >>>>> [email protected]> wrote: >>>>> >>>>>> The proxy-type issue is indeed the one wart in this project. I know >>>>>> that Chrome (at least on Windows, and probably everywhere) has decided >>>>>> not >>>>>> to enforce strict aliasing, but V8 may not have given up on that. >>>>>> >>>>>> The proxy types are rarely if ever read from or written to using the >>>>>> proxy type, but I'm not sure if that makes them legal or if that just >>>>>> means >>>>>> that every reference is UB - I suspect every reference is UB. And, I >>>>>> can't >>>>>> guarantee that they are only read/written using the real type - object >>>>>> copying would necessarily copy the bits using the proxy type. >>>>>> >>>>>> I don't think that windows.h can be removed without using this >>>>>> technique (not unless every one of these becomes a pointer to an >>>>>> allocated >>>>>> object). In the Chromium code-base I decided that the ugliness of the >>>>>> aliasing was less than the ugliness of windows-h-everywhere. V8 may, of >>>>>> course, come to a different conclusion. >>>>>> >>>>>> I updated my CL which uses the proxy technique on a couple of types >>>>>> so that it will build. That will let us see what the UBSan bots think of >>>>>> it. Is the regular dry run sufficient? >>>>>> >>>>>> On Thu, Jul 22, 2021 at 11:55 AM Leszek Swirski <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Hi Bruce, >>>>>>> >>>>>>> Thanks for looking into this, I'd be happy to do code reviews and >>>>>>> give advice if you're ok with the EMEA timezone delay - otherwise we've >>>>>>> a >>>>>>> couple of people in PST too. >>>>>>> >>>>>>> One question about the proxy types: are we at risk of triggering >>>>>>> undefined behaviour by reinterpret casting between the proxy types and >>>>>>> the >>>>>>> Windows types? We've gone to some lengths to be UBSan clean and I >>>>>>> wouldn't >>>>>>> want to regress that. >>>>>>> >>>>>>> Cheers, >>>>>>> Leszek >>>>>>> >>>>>>> >>>>>>> On Thu, 22 Jul 2021, 19:57 'Bruce Dawson' via v8-dev, < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> To make this more concrete I created a V8 CL >>>>>>>> <https://chromium-review.googlesource.com/c/v8/v8/+/3042215> that >>>>>>>> demonstrates what is involved, >>>>>>>> using obj/v8/v8_libbase/condition-variable.obj as the test case. That >>>>>>>> file >>>>>>>> builds. Other files do not. >>>>>>>> >>>>>>>> Some of the files are just hacked into compliance, but it gives a >>>>>>>> sense of how it would work. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Thu, Jul 22, 2021 at 10:23 AM Bruce Dawson < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> I'm a Chrome developer and I've been working on reducing how >>>>>>>>> frequently windows.h is included in source files when building Chrome >>>>>>>>> for >>>>>>>>> Windows. It used to be (2018) that more than 78% of Chrome's >>>>>>>>> translation >>>>>>>>> units included windows.h When I picked up this project again in June >>>>>>>>> this >>>>>>>>> had been reduced and about half of Chrome's translation units pulled >>>>>>>>> in >>>>>>>>> windows.h. Since June I have reduced this to less than 22%. About 9% >>>>>>>>> of the >>>>>>>>> remaining uses of windows.h (2% of source files used when building >>>>>>>>> Chrome) >>>>>>>>> are in v8 - 761 translation units. >>>>>>>>> >>>>>>>>> The advantages of reducing usage of windows.h are (very slightly) >>>>>>>>> reduced compile times, reduced namespace pollution (no more #define >>>>>>>>> DrawText DrawTextW), reduced warning-flag manipulation, etc. Mostly >>>>>>>>> it's >>>>>>>>> about avoiding those macro definitions. >>>>>>>>> >>>>>>>>> So... >>>>>>>>> >>>>>>>>> I'm looking at getting v8 to use windows.h less. The techniques >>>>>>>>> are well understood and proven and are best summarized in the >>>>>>>>> contents of >>>>>>>>> windows_types.h >>>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_types.h;l=9?q=windows_types.h&sq=>. >>>>>>>>> This creates the minimal set of typedefs and defines needed to compile >>>>>>>>> Chrome's header files and portable code. In a few cases >>>>>>>>> (CONDITION_VARIABLE, for instance) it is necessary to define Chrome >>>>>>>>> proxy's >>>>>>>>> for Windows types, and cast between them when calling Windows >>>>>>>>> functions >>>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_types.h;l=299?q=windows_types.h> >>>>>>>>> . >>>>>>>>> >>>>>>>>> This is a medium sized project that will require landing lots of >>>>>>>>> small CLs. I've done enough investigation to get a sense of the >>>>>>>>> scope. I'll >>>>>>>>> need to create V8 proxy-types for CONDITION_VARIABLE, SRWLOCK, and >>>>>>>>> CRITICAL_SECTION, create typedefs for HANDLE (this is easy - there's >>>>>>>>> no >>>>>>>>> casting required), and explicitly include windows.h in a few source >>>>>>>>> files >>>>>>>>> that actually need it. And I'll need to fix whatever other issues pop >>>>>>>>> up, >>>>>>>>> but I'm not expecting anything worse than what I've seen before. I >>>>>>>>> think >>>>>>>>> that the vast majority of v8 files will be able to compile without >>>>>>>>> windows.h. >>>>>>>>> >>>>>>>>> The overall project is tracked by crbug.com/796644 and the bugs >>>>>>>>> which block it. >>>>>>>>> >>>>>>>>> Is this a project that V8 would support? If so, can I get a >>>>>>>>> volunteer/sponsor who I can work with for advice and some code >>>>>>>>> reviews? >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Bruce Dawson, he/him >>>>>>>> >>>>>>>> -- >>>>>>>> -- >>>>>>>> v8-dev mailing list >>>>>>>> [email protected] >>>>>>>> http://groups.google.com/group/v8-dev >>>>>>>> --- >>>>>>>> You received this message because you are subscribed to the Google >>>>>>>> Groups "v8-dev" group. >>>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>>> send an email to [email protected]. >>>>>>>> To view this discussion on the web visit >>>>>>>> https://groups.google.com/d/msgid/v8-dev/CAE5mQiMF_Yx4cvVKtfhw1bGxqdKCx0U7%3DEz%2BMqGm%2BO%2BBG47U7w%40mail.gmail.com >>>>>>>> <https://groups.google.com/d/msgid/v8-dev/CAE5mQiMF_Yx4cvVKtfhw1bGxqdKCx0U7%3DEz%2BMqGm%2BO%2BBG47U7w%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>> . >>>>>>>> >>>>>>> -- >>>>>>> -- >>>>>>> v8-dev mailing list >>>>>>> [email protected] >>>>>>> http://groups.google.com/group/v8-dev >>>>>>> --- >>>>>>> You received this message because you are subscribed to a topic in >>>>>>> the Google Groups "v8-dev" group. >>>>>>> To unsubscribe from this topic, visit >>>>>>> https://groups.google.com/d/topic/v8-dev/MbE-A6QBOCM/unsubscribe. >>>>>>> To unsubscribe from this group and all its topics, send an email to >>>>>>> [email protected]. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/d/msgid/v8-dev/CAGRskv-kw1aFXyK%2BQwmOffvh3TVZvWgHHw6MBpoRMeGhGTJ2Hg%40mail.gmail.com >>>>>>> <https://groups.google.com/d/msgid/v8-dev/CAGRskv-kw1aFXyK%2BQwmOffvh3TVZvWgHHw6MBpoRMeGhGTJ2Hg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Bruce Dawson, he/him >>>>>> >>>>>> -- >>>>>> -- >>>>>> v8-dev mailing list >>>>>> [email protected] >>>>>> http://groups.google.com/group/v8-dev >>>>>> --- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "v8-dev" group. >>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>> send an email to [email protected]. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/d/msgid/v8-dev/CAE5mQiMsatJLmHW9Aufq5EVob3DYQCYa8h_xs7-bfVBLXngOjA%40mail.gmail.com >>>>>> <https://groups.google.com/d/msgid/v8-dev/CAE5mQiMsatJLmHW9Aufq5EVob3DYQCYa8h_xs7-bfVBLXngOjA%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>> . >>>>>> >>>>> -- >>>> -- >>>> v8-dev mailing list >>>> [email protected] >>>> http://groups.google.com/group/v8-dev >>>> --- >>>> You received this message because you are subscribed to a topic in the >>>> Google Groups "v8-dev" group. >>>> To unsubscribe from this topic, visit >>>> https://groups.google.com/d/topic/v8-dev/MbE-A6QBOCM/unsubscribe. >>>> To unsubscribe from this group and all its topics, send an email to >>>> [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/v8-dev/CAGRskv9h3KKW59O4VswHZhCkSdPg-wx6JLy0Mc7jkDw640woWg%40mail.gmail.com >>>> <https://groups.google.com/d/msgid/v8-dev/CAGRskv9h3KKW59O4VswHZhCkSdPg-wx6JLy0Mc7jkDw640woWg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>> >>> >>> -- >>> Bruce Dawson, he/him >>> >>> -- >>> -- >>> v8-dev mailing list >>> [email protected] >>> http://groups.google.com/group/v8-dev >>> --- >>> You received this message because you are subscribed to the Google >>> Groups "v8-dev" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/v8-dev/CAE5mQiPtfWwttimwqxoMVkT0aO8q-hjSLCeZgoFDY0jjO62STg%40mail.gmail.com >>> <https://groups.google.com/d/msgid/v8-dev/CAE5mQiPtfWwttimwqxoMVkT0aO8q-hjSLCeZgoFDY0jjO62STg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- >> -- >> v8-dev mailing list >> [email protected] >> http://groups.google.com/group/v8-dev >> --- >> You received this message because you are subscribed to a topic in the >> Google Groups "v8-dev" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/v8-dev/MbE-A6QBOCM/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/v8-dev/CALH_77tLNROkW7dDzj%3Dg4HK%2BMk%3DrZKRgYP%3Dq%3DTWtkMaeb1-Sow%40mail.gmail.com >> <https://groups.google.com/d/msgid/v8-dev/CALH_77tLNROkW7dDzj%3Dg4HK%2BMk%3DrZKRgYP%3Dq%3DTWtkMaeb1-Sow%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > > > -- > Bruce Dawson, he/him > > -- Bruce Dawson, he/him -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAE5mQiOKzh5K5VEq189JK88t3mY-ip0o6909pb6s61aeqw0a4w%40mail.gmail.com.
