From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 45C733840C09 for ; Thu, 28 May 2020 19:00:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 45C733840C09 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-309-fbI4Q2-DPhyeFsYxTzqNMA-1; Thu, 28 May 2020 15:00:39 -0400 X-MC-Unique: fbI4Q2-DPhyeFsYxTzqNMA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A2EF5461; Thu, 28 May 2020 19:00:38 +0000 (UTC) Received: from ovpn-112-12.phx2.redhat.com (ovpn-112-12.phx2.redhat.com [10.3.112.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2A4CA1038; Thu, 28 May 2020 19:00:37 +0000 (UTC) Message-ID: <885d0b34df56c30f25c2ba57f4eecf517d1ba05c.camel@redhat.com> Subject: Re: [PATCH] Port libgccjit to Windows. From: David Malcolm To: Nicolas =?ISO-8859-1?Q?B=E9rtolo?= Cc: jit@gcc.gnu.org, gcc-patches@gcc.gnu.org Date: Thu, 28 May 2020 15:00:37 -0400 In-Reply-To: References: <4b619179a08075bd2ee7f9e98aa2d5918191306d.camel@redhat.com> <5de2a5202b50882612e1fe51d254f2b125f61716.camel@redhat.com> User-Agent: Evolution 3.32.5 (3.32.5-1.fc30) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 May 2020 19:00:54 -0000 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)