On Wed, 2020-05-27 at 22:27 -0300, Nicolas Bértolo wrote: > Hi, > > > Do you have commit/push access to the gcc repository? > > No I don't. > > > BTW, why isn't it necessary to use --enable-host-shared in Windows? > > Can we document that? > > That's because all code is position independent in Windows. > > > On the subject of nitpicking, I find myself getting distracted by > the > > indentation in the patch; there seem to be a lot of mismatches. > > > What editor are you using, and does it have options to > > (a) show visible whitespace, and > > (b) to apply a formatting convention? > > > I use Emacs, and it takes care of this for me. I haven't used it, > but > > there's a contrib/clang-format file in the gcc source tree which > > presumably describes GCC's coding conventions, if that helps for > the > > new code. > > The problem seems to be that I was writing tabs but since I have set > up my > editor to show them as 2 spaces I couldn't see what was wrong.
Thanks; the latest patch is much better. > > Am I right in thinking that this installs the libgccjit.a file on > Windows? > > Why is this done? > > That is the file libgccjit.dll.a > > It is the import library for gccjit. It is part of the way Windows > handles > dynamic libraries. Thanks. > > New C++ source files should have a .cc extension. > > I hope that at some point we'll rename all the existing .c ones > > accordingly. > > I just couldn't get Make to generate jit-w32.o from jit-w32.cc. > It looks for jit-w32.c. > > I had to leave it with the .c extension. Fair enough. > > Does this call generate a directory that's only accessible to the > > current user? > > Otherwise there could be a risk of a hostile user on the same > machine > > clobbering the contents and injecting code into this process. > > I changed the code to generate a directory than can only be accessed > by the > current user. > > I've attached a new version. It contains a rewrite of the code that > creates > temporary directories. > > Nico I'm going to have to trust your Windows expertise here; the tempdir code looks convoluted to me, but perhaps that's the only way to do it. (Microsoft's docs for "SECURITY_ATTRIBUTES" suggest to me that if lpSecurityDescriptor is NULL, then the directory gets a default security descriptor, and that this may mean it's only readable by the user represented by the access token of the process [1], which might suggest a simplification - but I'm very hazy on how the security model in Windows works) I was able to successfully bootstrap and regression test with your patch on x86_64-pc-linux-gnu. I also verified that the result of "make install" was not affected for my configuration. I've pushed your patch to master as c83027f32d9cca84959c7d6a1e519a0129731501. (I had to do a little fixup of the ChangeLog entries to get them to work with the new hooks on our git repo) Thanks again for the patch Dave [1] https://docs.microsoft.com/en-us/previous-versions/windows/desktop/legacy/aa379560(v=vs.85)
