From nobody Fri Jan 17 02:53:07 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YZ46Y2dlzz5kJ1L; Fri, 17 Jan 2025 02:53:09 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YZ46Y22KCz3fBl; Fri, 17 Jan 2025 02:53:09 +0000 (UTC) (envelope-from kevans@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737082389; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DtWuQJMYKWZLkde+2M8Hm8XcKhsOm7Oq9JQZ1kYzJLw=; b=KwX0S5Iu/pTqnnezz4rx7UOMIvr3Nbvfn8+fHRyo9Ej7SqTo8Psv4cFBKVzDgPSrZWhe1s X2evwHb+p3/RxsTluREGSjlUiQW8jQZIQiCGWsI/gGGt7qrha79M7s5dgZ9viRfETur0QZ 6BkybxVcJEInMFixrJbaEMptVkRWJO1HZvF/WMjwEo41fKSaLtmba+C5JsqTSrbM8lC9Oe +IyCuey3ErGWk1iBW6Ep5SDq08Yxn1W3MlOTgFg3GPhK2rzGH6KJonPkYw8Crj82bIXEV4 Y2DBtB0GANRUR1+PPzrq4+0FghXOB8LA4f41oCWpSzWPWCy2RKbvuKfZBB26Jw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737082389; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DtWuQJMYKWZLkde+2M8Hm8XcKhsOm7Oq9JQZ1kYzJLw=; b=ZpHf026bfj+8F2lflbr7InpnSTZyRzY6hYX3y9GTY92NWEVnIuBGzRtdJcBqrHdJRxXhAj 2BZSy8AcK7mVJZWXGKM958+OET5Px6zpYYBGf4SgI1q55M7MZO/+yyHTDXPZ24hWEO0rOb BQp98UzCX54Wv2fpYNWHB5InT5QN7MIZKp9pobi0w9CdjdFl40cpLR+3zTkEJ96EX4yf/2 somz48G50qPeGhlSUkvmnFTMYn0UahtI0N+SHRAw8+BpmGVjGmyoOHon5dwTnP2jJOdm/y JqsZtjQXpP+TKFlG7SaWDZPvrH0rLO6oKINEcNLxmzg4hDbWk0NT5la7agyKJg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737082389; a=rsa-sha256; cv=none; b=YKpBL2Z36SA7v8MwHGMFiFc8X7XJaQ/OGmgVkN5dmU5tqu5pwhjVWa0p0EVnJS82DzTumZ p19Z2yWWx2ndgMSugV7BJh9g6zKWEf/Fu3Q7qzse2aFWYc6LoQek5RDuQKwhcDeZRSwc5A hWhdxJT7yDDyZn3xL5aSNmMC/hOrh1fkXKrsufcgcfTsh+NYC612UHizIonXedhN+lR4i7 RYN+6SfQ/+BGW6BRjSP5Buk04rndNBkwQfkId321jFCJRv98Rs2HzIjBXjh8Y5UJWRVD2N 9Q7Lj5MdhkhpCa0oGsiepbls2zEoKTHPPmkEr33tMY4srmHgxLrPog/p9trgKg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from [10.9.4.95] (unknown [209.182.120.176]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: kevans/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4YZ46X5lmPz154Q; Fri, 17 Jan 2025 02:53:08 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Message-ID: Date: Thu, 16 Jan 2025 20:53:07 -0600 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: 9ded074e875c - main - Refactor makesyscalls.lua into a library To: Brooks Davis Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, Warner Losh References: <202410302108.49UL8tGl053622@gitrepo.freebsd.org> <25a28def-fbfd-49df-a2bf-dc4ef6609440@FreeBSD.org> Content-Language: en-US From: Kyle Evans In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 1/16/25 16:43, Brooks Davis wrote: > On Thu, Jan 16, 2025 at 03:52:22PM -0600, Kyle Evans wrote: >> On 10/30/24 16:08, Brooks Davis wrote: >>> The branch main has been updated by brooks: >>> >>> URL: https://cgit.FreeBSD.org/src/commit/?id=9ded074e875c29cb92d5f643801990d7bb23cca4 >>> >>> commit 9ded074e875c29cb92d5f643801990d7bb23cca4 >>> Author: agge3 >>> AuthorDate: 2024-10-21 21:42:13 +0000 >>> Commit: Brooks Davis >>> CommitDate: 2024-10-30 21:04:30 +0000 >>> >>> Refactor makesyscalls.lua into a library >>> * main.lua replicates the functionality of makesyscalls.lua >>> * Individual files are generated by their associated module >>> * Modules can be called as standalone scripts to generate a specific >>> file >>> * Data and procedures are performed by objects instead of procedual code >>> * Bitmasks are replaced by declarative types >>> * Temporary files are no longer produced, writing is stored in memory >>> * Comments provide explanation to functions and semantics >>> Google Summer of Code 2024 Final Work Product >>> Co-authored-by: Warner Losh >>> Co-authored-by: Kyle Evans >>> Co-authored-by: Brooks Davis >>> Sponsored by: Google (GSoC 24) >>> Pull Request: https://github.com/freebsd/freebsd-src/pull/1362 >>> Signed-off-by: agge3 >>> --- >>> sys/tools/syscalls/README.md | 49 +++ >>> sys/tools/syscalls/config.lua | 312 +++++++++++++++++ >>> sys/tools/syscalls/core/freebsd-syscall.lua | 147 ++++++++ >>> sys/tools/syscalls/core/scarg.lua | 163 +++++++++ >>> sys/tools/syscalls/core/scret.lua | 45 +++ >>> sys/tools/syscalls/core/syscall.lua | 497 +++++++++++++++++++++++++++ >>> sys/tools/syscalls/main.lua | 64 ++++ >>> sys/tools/syscalls/scripts/init_sysent.lua | 193 +++++++++++ >>> sys/tools/syscalls/scripts/libsys_h.lua | 111 ++++++ >>> sys/tools/syscalls/scripts/syscall_h.lua | 97 ++++++ >>> sys/tools/syscalls/scripts/syscall_mk.lua | 90 +++++ >>> sys/tools/syscalls/scripts/syscalls.lua | 109 ++++++ >>> sys/tools/syscalls/scripts/syscalls_map.lua | 74 ++++ >>> sys/tools/syscalls/scripts/sysproto_h.lua | 242 +++++++++++++ >>> sys/tools/syscalls/scripts/systrace_args.lua | 268 +++++++++++++++ >>> sys/tools/syscalls/tools/generator.lua | 113 ++++++ >>> sys/tools/syscalls/tools/util.lua | 194 +++++++++++ >>> 17 files changed, 2768 insertions(+) >>> >>> [...] >>> diff --git a/sys/tools/syscalls/core/freebsd-syscall.lua b/sys/tools/syscalls/core/freebsd-syscall.lua >>> new file mode 100644 >>> index 000000000000..193b1e43563c >>> --- /dev/null >>> +++ b/sys/tools/syscalls/core/freebsd-syscall.lua >>> @@ -0,0 +1,147 @@ >>> [...] >>> +function FreeBSDSyscall:parseSysfile() >>> + local file = self.sysfile >>> + local config = self.config >>> + local commentExpr = "^%s*;.*" >>> + >>> + if file == nil then >>> + return nil, "No file given" >>> + end >>> + >>> + self.syscalls = {} >>> + >>> + local fh, msg = io.open(file) >>> + if fh == nil then >>> + return nil, msg >>> + end >>> + >>> + local incs = "" >>> + local defs = "" >>> + local s >>> + for line in fh:lines() do >>> + line = line:gsub(commentExpr, "") -- Strip any comments. >>> + -- NOTE: Can't use pure pattern matching here because of >>> + -- the 's' test and this is shorter than a generic pattern >>> + -- matching pattern. >>> + if line == nil or line == "" then >>> + goto skip -- Blank line, skip this line. >>> + elseif s ~= nil then >>> + -- If we have a partial system call object s, >>> + -- then feed it one more line. >>> + if s:add(line) then >>> + -- Append to system call list. >>> + for t in s:iter() do >>> + if t:validate(t.num - 1) then >>> + table.insert(self.syscalls, t) >>> + else >>> + util.abort(1, >>> + "Skipped system call " .. >>> + "at number " .. t.num) >>> + end >>> + end >>> + s = nil >>> + end >>> + elseif line:match("^#%s*include") then >>> + incs = incs .. line .. "\n" >>> + elseif line:match("%%ABI_HEADERS%%") then >>> + local h = self.config.abi_headers >>> + if h ~= nil and h ~= "" then >>> + incs = incs .. h .. "\n" >>> + end >>> + elseif line:match("^#%s*define") then >>> + defs = defs .. line.. "\n" >>> + elseif line:match("^#") then >>> + util.abort(1, "Unsupported cpp op " .. line) >> >> This specifically is kind of a huge regression, and I don't really know how to >> cope with it. We've guaranteed for years that we'll copy preprocessor >> directives through to all output files. We don't use that upstream in >> FreeBSD, but we work with downstreams/vendors that make extensive use of it in >> their syscall definitions. >> >> I don't really know what the answer is to this, but we probably shouldn't have >> dropped it without some discussion first. This is going to be a bit of a >> headache... > > This response seems rather hyperbolic. This change was up for review > for months and the feature is unused in tree so there was no way to know > it was important. > Re-reading, yes, this was a bit dramatic; my apologies. There's plenty of frustration here, mostly amplified by the fact that I was on the review just as much as you folks and have worked in environments that use it- it certainly should have stuck out to me, but I just didn't have the time into it that I'd hoped I would. I would've also really liked to see an "XXX" comment at a minimum drawing attention to it or a call-out in the commit message, given that the syscall definition documentation isn't that lengthy and this is one of the few guarantees we make it. I think there's some compromise to be had, but... > It would be helpful to work through some examples understand what people > need here and if it really has to be a refactor to pass things through > or if adding some new tags and config values could do the job. > ... I'll respond to this this weekend, hopefully. I'd like to condense what I'm aware of into some formal test cases in sys/tools that I can point to so that we have something less abstract to debate the merits of, and also so that we have something we can verify the functionality against. > The quickest workaround would probably to revert these commits locally > (I took care to isolate them for exactly this reason): > > 5212b9500116 sysent: GC sys/tools/makesyscalls.lua > 204d065dac81 sysent: switch to refactored makesyscalls.lua > > You might also need to revert this commit or implement it in > makesyscalls.lua: > > bbc0f33b1317 sysent: add a NOLIB modifer to prevent stub generation > > If we need be, I'd even be ok with temporarily restoring makesyscalls.lua > and back porting easy features, but maintaining two is certainly not > viable indefinitely. > This isn't urgent enough to warrant any of that- it's just something that I'd like to work out how we can solve (preferably in the short-to-mid term), because there good uses for at least a subset of what we supported before. I don't think we need to go as far as supporting arbitrary preprocessor, but if we could at least devise something for conditional syscall slots that'd probably be sufficient and workable. Thanks, Kyle Evans