From nobody Mon Jan 20 01:41:32 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 4YbtNZ4BNKz5k2rb; Mon, 20 Jan 2025 01:41:34 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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 4YbtNZ3T8sz3kD7; Mon, 20 Jan 2025 01:41:34 +0000 (UTC) (envelope-from kevans@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737337294; 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=XZbKYL0dMtbw3XBXPhFP7lP7WXAH3CIEjfLjM/h/EXM=; b=xCVrv4Ah4o9b94N/3Q83Br2ZO/q7n3dFN5/lFpWVATjs9xx3L8NTQ8W19UaXONc1VCm0K1 lBDW7XIYTTmWJwX/z/4sp59IQlrOfmjQ03wZ2SSQy9tsvV7tS1QVxtDQe+v6/4eTgAbSIe 4phl+uabOXhiER3coHenVmkpNzfZf2V9eWpQEEVB768rAKNCZqAulw8eSEkWSS28zSzlzA smB4klS/8ygtxFu90xVdobVnxLWosAZrI/cllIkLQ2X0awOWZ+rm3PLxW/bnNPa42MoVwA Utsp55sUKhIHmYOOm1mnCSzZVQUyLrseWdZm453fs7Q9zokTT5hc4PrjLEAHdQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737337294; 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=XZbKYL0dMtbw3XBXPhFP7lP7WXAH3CIEjfLjM/h/EXM=; b=wcP0vWNW/OgKkZBH6LL5NOyERe5kXNMtDC2Pqr49mV8y2C5/SenozrURuca72XjPZe3G62 +JopzVD7pbadv2qI/v8xtzuJqSwBzczNgm0LCUmUBHHLNq5k2w3087ojqNsKL1tlSeGfeD 87vTj8ddtKpoMMLO7EFwOBc0tDqPsTVfKJEep2rIXpKJkPsu78IO9TivU6Wzefwuzgq4ao KcO5raPY/QhC8tBImbX9YgKa2oZQfxh+P26SXMdsb4qS61Y15BEkJ5JRiCs2Sbp9ZvfEia y904Rpzt8lTEMvuZdZi1NjMmwPkvhHacCet1qyUvpRTehm1vM/XjJ6S6Mt6fNA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737337294; a=rsa-sha256; cv=none; b=qCxktLhCrlSuDSrFBgGa7q98HbQNizhGDVui1/R2+kNDx5+q8QAN157l3EDLn9x5AtnCy7 6OLcUURndQIbNebEku+eKAxM2YiXppUNN6IQj3kxVRlprcYAXHG5ZTfI/n35t2v36KcOTb Iuvfww+pI8Stg7etLjR2wfMop2llNt/fLEpchYdaVhrmIPCHgalV+xq56tWW7t40K2LYrw RgR/7Utz05vV6sBNFtdj1GGgaBHAFloi3rTtu8mArjRZSS674AdbJsXeMa6kxQpfqSfR0n qViNcGVQUSTnJsdVkHAqqY5wDK+O8VM65s5EFT5mxutwkiOd18atP6H2cjoCRA== 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 4YbtNZ04myzQ6f; Mon, 20 Jan 2025 01:41:33 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Message-ID: <7371f49b-125a-47af-b36b-38f3a8ebfef1@FreeBSD.org> Date: Sun, 19 Jan 2025 19:41:32 -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/17/25 12:35, Brooks Davis wrote: > On Thu, Jan 16, 2025 at 08:53:07PM -0600, Kyle Evans wrote: >> 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. > > I've implemented simple support for ifdef's syscall variants in > https://github.com/freebsd/freebsd-src/pull/1575. It's not robust at all > and may be missing some bits in newly generated files, but it's probably > not much worse than what's in 14. > I'll take a look when I get a chance, thanks. > I agree some tests would be good. I think I'd deviate from the norm and > run them as part of the top level `make sysent` for ease of develoment. I took a stab at this here: https://github.com/kevans91/freebsd/commit/083215d48541eb2be5e7725031c319f50f3881c8 Most of the uses are fairly trivial but, IMO, pretty reasonable (and generally of the same pattern). Even just a subset of CPP parsing to cover trivial #ifdef / #else / #endif sequences and tagging syscalls with a `condition`` that propagates to generated files would be sufficient, even if we aren't blindly propagating preprocessor conditions anymore. Thanks, Kyle Evans