implements: update modernc.org/cc pkg to v4 #1142

Nikhil-Ladha · 2025-06-18T06:25:07Z

Update modernc.org/cc pkg to v4 and make required api changes to the cast script for metadata generation

Checklist

Added tests for features and functional changes
Public functions and types are documented
Standard formatting is applied to Go code
Is this a new API? Added a new file that begins with //go:build ceph_preview
Ran make api-update to record new APIs

New or infrequent contributors may want to review the go-ceph Developer's Guide including the section on how we track API Status and the API Stability Plan.

The go-ceph project uses mergify. View the mergify command guide for information on how to interact with mergify. Add a comment with @Mergifyio rebase to rebase your PR when github indicates that the PR is out of date with the base branch.

Nikhil-Ladha · 2025-06-18T06:48:50Z

Any ideas/suggestions on what could be the reason for this error?

*** running: ./implements --list --report-json /results/implements.json --report-text /results/implements.txt cephfs cephfs/admin common/admin/manager common/admin/nfs common/admin/smb common/commands common/log rados rados/striper rbd rbd/admin rgw rgw/admin
2025/06/18 06:44:52 error: /usr/include/sys/cdefs.h:30:3: "You need a ISO C conforming compiler to use the glibc headers"
/usr/include/sys/types.h:144:10: include file not found: <stddef.h>

Nikhil-Ladha · 2025-06-18T08:10:41Z

/cc @anoopcs9 @phlogistonjohn @ansiwen

anoopcs9 · 2025-06-18T11:23:49Z

Any ideas/suggestions on what could be the reason for this error?

*** running: ./implements --list --report-json /results/implements.json --report-text /results/implements.txt cephfs cephfs/admin common/admin/manager common/admin/nfs common/admin/smb common/commands common/log rados rados/striper rbd rbd/admin rgw rgw/admin
2025/06/18 06:44:52 error: /usr/include/sys/cdefs.h:30:3: "You need a ISO C conforming compiler to use the glibc headers"
/usr/include/sys/types.h:144:10: include file not found: <stddef.h>

I think you should go for cc.NewConfig(). Then there is a caveat in using cc.Parse() where it does not source anything by default. With that I think the following additional change should get us going..

index bc65f5a..c05c1de 100644
--- a/contrib/implements/internal/implements/cast.go
+++ b/contrib/implements/internal/implements/cast.go
@@ -2,6 +2,7 @@ package implements
 
 import (
        "fmt"
+       "runtime"
        "strings"
 
        "modernc.org/cc/v4"
@@ -66,16 +67,18 @@ func stubCFunctions(libname string) (CFunctions, error) {
        if cstub == "" {
                return nil, fmt.Errorf("no C stub available for '%s'", libname)
        }
-       conf := cc.Config{
-               IncludePaths:    []string{"@", "/usr/local/include", "/usr/include"},
-               SysIncludePaths: []string{"@", "/usr/local/include", "/usr/include"},
+       conf, err := cc.NewConfig(runtime.GOOS, runtime.GOARCH, "-D_FILE_OFFSET_BITS=64")
+       if err != nil {
+               return nil, err
        }
 
        src := []cc.Source{
+               {Name: "predefined", Value: conf.Predefined},
+               {Name: "builtin", Value: cc.Builtin},
                {Name: "typestubs", Value: typeStubs},
                {Name: libname, Value: cstub},
        }
-       cAST, err := cc.Parse(&conf, src)
+       cAST, err := cc.Parse(conf, src)
        if err != nil {
                return nil, err
        }

Nikhil-Ladha · 2025-06-18T15:36:37Z

I think you should go for cc.NewConfig(). Then there is a caveat in using cc.Parse() where it does not source anything by default. With that I think the following additional change should get us going..

index bc65f5a..c05c1de 100644
--- a/contrib/implements/internal/implements/cast.go
+++ b/contrib/implements/internal/implements/cast.go
@@ -2,6 +2,7 @@ package implements
 
 import (
        "fmt"
+       "runtime"
        "strings"
 
        "modernc.org/cc/v4"
@@ -66,16 +67,18 @@ func stubCFunctions(libname string) (CFunctions, error) {
        if cstub == "" {
                return nil, fmt.Errorf("no C stub available for '%s'", libname)
        }
-       conf := cc.Config{
-               IncludePaths:    []string{"@", "/usr/local/include", "/usr/include"},
-               SysIncludePaths: []string{"@", "/usr/local/include", "/usr/include"},
+       conf, err := cc.NewConfig(runtime.GOOS, runtime.GOARCH, "-D_FILE_OFFSET_BITS=64")
+       if err != nil {
+               return nil, err
        }
 
        src := []cc.Source{
+               {Name: "predefined", Value: conf.Predefined},
+               {Name: "builtin", Value: cc.Builtin},
                {Name: "typestubs", Value: typeStubs},
                {Name: libname, Value: cstub},
        }
-       cAST, err := cc.Parse(&conf, src)
+       cAST, err := cc.Parse(conf, src)
        if err != nil {
                return nil, err
        }

Thanks, that worked.
The only difference was it would be <predefined> and <builtin>

phlogistonjohn · 2025-06-18T16:49:01Z

What I would like to know is what's the motivation for this change? Is there something not working with the tool or is it just a desire to use a newer version of the library?

anoopcs9 · 2025-06-18T17:41:33Z

Thanks, that worked. The only difference was it would be <predefined> and <builtin>

Ah..my bad.

Nikhil-Ladha · 2025-06-18T18:00:48Z

What I would like to know is what's the motivation for this change? Is there something not working with the tool or is it just a desire to use a newer version of the library?

Just a desire to use a newer version of the library as I was trying to fix old CVEs related to the previous version and I saw that the previous version is no longer maintained, so how about we upgrade to the latest version and be up-to-date with the latest releases.
P.S: It would also open the way for the recent dependabot changes to upgrade this pkg when newer versions are available.

contrib/implements/go.mod

phlogistonjohn · 2025-06-18T19:19:32Z

Sounds reasonable. Thanks for clarifying.

Nikhil-Ladha · 2025-06-20T06:07:33Z

If everything looks good, can we get this merged?

contrib/implements/internal/implements/cast.go

Nikhil-Ladha · 2025-06-23T12:20:27Z

@anoopcs9 updated the code based on the discussion over slack, PTAL.

anoopcs9

Still implements.txt (or implements.json) doesn't match. One difference is with librbd.h where the deprecated attribute is indirectly used with the help of another macro. Even otherwise somehow all standard system calls are listed which was not the case before. Unfortunately we need more investigation and changes.

contrib/implements/internal/implements/cast.go

anoopcs9 · 2025-06-25T08:30:45Z

Even otherwise somehow all standard system calls are listed which was not the case before.

The following additional check should get rid of those builtin functions:

index c810a4c..89ccc00 100644
--- a/contrib/implements/internal/implements/cast.go
+++ b/contrib/implements/internal/implements/cast.go
@@ -3,6 +3,7 @@ package implements
 import (
        "fmt"
        "runtime"
+       "strings"
 
        "modernc.org/cc/v4"
 )
@@ -62,6 +63,7 @@ func stubCFunctions(libname string) (CFunctions, error) {
                for _, n := range cAST.Scope.Nodes[i] {
                        if n, ok := n.(*cc.Declarator); ok &&
                                !n.IsTypename() &&
+                               !strings.Contains(n.String(), "<builtin>") &&
                                n.DirectDeclarator != nil &&
                                (n.DirectDeclarator.Case == cc.DirectDeclaratorFuncParam ||
                                        n.DirectDeclarator.Case == cc.DirectDeclaratorFuncIdent) {

Nikhil-Ladha · 2025-06-25T09:49:51Z

Even otherwise somehow all standard system calls are listed which was not the case before.

The following additional check should get rid of those builtin functions:

index c810a4c..89ccc00 100644
--- a/contrib/implements/internal/implements/cast.go
+++ b/contrib/implements/internal/implements/cast.go
@@ -3,6 +3,7 @@ package implements
 import (
        "fmt"
        "runtime"
+       "strings"
 
        "modernc.org/cc/v4"
 )
@@ -62,6 +63,7 @@ func stubCFunctions(libname string) (CFunctions, error) {
                for _, n := range cAST.Scope.Nodes[i] {
                        if n, ok := n.(*cc.Declarator); ok &&
                                !n.IsTypename() &&
+                               !strings.Contains(n.String(), "<builtin>") &&
                                n.DirectDeclarator != nil &&
                                (n.DirectDeclarator.Case == cc.DirectDeclaratorFuncParam ||
                                        n.DirectDeclarator.Case == cc.DirectDeclaratorFuncIdent) {

Thanks, in the meantime I got us a fix for the librbd issue where the deprecated macro was defined in a different way.
Ref: https://gitlab.com/cznic/cc/-/commit/3ffe6a48d7d48fc2e41ae5239fc89ebe4b1d2aa4, the fix should be available in a release in a couple of days.

anoopcs9 · 2025-06-25T11:17:28Z

Thanks, in the meantime I got us a fix for the librbd issue where the deprecated macro was defined in a different way. Ref: https://gitlab.com/cznic/cc/-/commit/3ffe6a48d7d48fc2e41ae5239fc89ebe4b1d2aa4, the fix should be available in a release in a couple of days.

Cool, with that and #1145 I could match the contents for implements.txt using the following change (there's a variation from my previous suggestion in #1142 (comment)):

index fe75794..0681213 100644
--- a/contrib/implements/internal/implements/cast.go
+++ b/contrib/implements/internal/implements/cast.go
@@ -3,6 +3,7 @@ package implements
 import (
 	"fmt"
 	"runtime"
+	"strings"
 
 	"modernc.org/cc/v4"
 )
@@ -63,14 +64,15 @@ func stubCFunctions(libname string) (CFunctions, error) {
 		for _, n := range cAST.Scope.Nodes[i] {
 			if n, ok := n.(*cc.Declarator); ok &&
 				!n.IsTypename() &&
+				strings.HasPrefix(n.Name(), funcPrefix[libname]) &&
 				n.DirectDeclarator != nil &&
 				(n.DirectDeclarator.Case == cc.DirectDeclaratorFuncParam ||
 					n.DirectDeclarator.Case == cc.DirectDeclaratorFuncIdent) {
 				name := n.Name()
 				if _, exists := cfMap[name]; !exists {
 					isDeprecated := false
-					if n.Type().Attributes().IsAttrSet("deprecated") {
-						isDeprecated = true
+					if attrs := n.Type().Attributes(); attrs != nil {
+						isDeprecated = attrs.IsAttrSet("deprecated")
 					}
 					cf := CFunction{
 						Name:         name,

Nikhil-Ladha · 2025-06-25T11:41:34Z

Thanks, in the meantime I got us a fix for the librbd issue where the deprecated macro was defined in a different way. Ref: https://gitlab.com/cznic/cc/-/commit/3ffe6a48d7d48fc2e41ae5239fc89ebe4b1d2aa4, the fix should be available in a release in a couple of days.

Cool, with that and #1145 I could match the contents for implements.txt using the following change (there's a variation from my previous suggestion in #1142 (comment)):

index fe75794..0681213 100644
--- a/contrib/implements/internal/implements/cast.go
+++ b/contrib/implements/internal/implements/cast.go
@@ -3,6 +3,7 @@ package implements
 import (
 	"fmt"
 	"runtime"
+	"strings"
 
 	"modernc.org/cc/v4"
 )
@@ -63,14 +64,15 @@ func stubCFunctions(libname string) (CFunctions, error) {
 		for _, n := range cAST.Scope.Nodes[i] {
 			if n, ok := n.(*cc.Declarator); ok &&
 				!n.IsTypename() &&
+				strings.HasPrefix(n.Name(), funcPrefix[libname]) &&
 				n.DirectDeclarator != nil &&
 				(n.DirectDeclarator.Case == cc.DirectDeclaratorFuncParam ||
 					n.DirectDeclarator.Case == cc.DirectDeclaratorFuncIdent) {
 				name := n.Name()
 				if _, exists := cfMap[name]; !exists {
 					isDeprecated := false
-					if n.Type().Attributes().IsAttrSet("deprecated") {
-						isDeprecated = true
+					if attr := n.Type().Attributes(); attr != nil {
+						isDeprecated = attr.IsAttrSet("deprecated")
 					}
 					cf := CFunction{
 						Name:         name,

Ack, then I will wait for your PR to be merged and the new release for the cc pkg to be available and then rebase the PR.
Or, is it fine if I use a commit to reference the cc pkg?

anoopcs9 · 2025-06-25T12:56:15Z

Ack, then I will wait for your PR to be merged and the new release for the cc pkg to be available and then rebase the PR. Or, is it fine if I use a commit to reference the cc pkg?

Let's wait for the next release.

Nikhil-Ladha · 2025-06-26T16:13:51Z

@anoopcs9 updated and rebased the PR with the newer version of pkg which contains the fix, PTAL.

anoopcs9

lgtm, thanks for the perseverance 😊.

Nikhil-Ladha · 2025-07-07T05:12:36Z

@ansiwen can you PTAL and add your review/comments?

ansiwen

LGTM, (although I didn't dive deep into the changes of the v4 API now). Thanks a lot @Nikhil-Ladha and @anoopcs9 for the hard work on this!

anoopcs9 · 2025-07-08T13:15:19Z

@Mergifyio rebase

update modernc.org/cc pkg to v4 and make required api changes to the cast script for metadata generation Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>

mergify · 2025-07-08T13:15:28Z

rebase

✅ Branch has been successfully rebased

Nikhil-Ladha force-pushed the update-modernc-pkg branch from 8bc14b7 to cc3a596 Compare June 18, 2025 15:35

anoopcs9 reviewed Jun 18, 2025

View reviewed changes

contrib/implements/go.mod Outdated Show resolved Hide resolved

Nikhil-Ladha force-pushed the update-modernc-pkg branch from cc3a596 to 95a4ba1 Compare June 19, 2025 06:01

Nikhil-Ladha requested a review from anoopcs9 June 19, 2025 10:11

anoopcs9 requested changes Jun 21, 2025

View reviewed changes

contrib/implements/internal/implements/cast.go Outdated Show resolved Hide resolved

contrib/implements/internal/implements/cast.go Outdated Show resolved Hide resolved

contrib/implements/internal/implements/cast.go Outdated Show resolved Hide resolved

Nikhil-Ladha force-pushed the update-modernc-pkg branch from 95a4ba1 to b7bd72e Compare June 23, 2025 11:47

Nikhil-Ladha requested a review from anoopcs9 June 23, 2025 12:20

anoopcs9 requested changes Jun 24, 2025

View reviewed changes

contrib/implements/internal/implements/cast.go Show resolved Hide resolved

Nikhil-Ladha force-pushed the update-modernc-pkg branch from b7bd72e to 8caafb9 Compare June 26, 2025 16:12

Nikhil-Ladha requested a review from anoopcs9 June 26, 2025 16:13

anoopcs9 approved these changes Jun 27, 2025

View reviewed changes

anoopcs9 added the extended-review A submitter or reviewer feels the PR needs an extended review period label Jun 27, 2025

anoopcs9 requested a review from ansiwen June 27, 2025 05:57

anoopcs9 added the no-API This PR does not include any changes to the public API of a go-ceph package label Jun 27, 2025

ansiwen approved these changes Jul 8, 2025

View reviewed changes

implements: update modernc.org/cc pkg to v4

78fd58f

update modernc.org/cc pkg to v4 and make required api changes to the cast script for metadata generation Signed-off-by: Nikhil-Ladha <nikhilladha1999@gmail.com>

anoopcs9 force-pushed the update-modernc-pkg branch from 8caafb9 to 78fd58f Compare July 8, 2025 13:15

anoopcs9 removed the extended-review A submitter or reviewer feels the PR needs an extended review period label Jul 8, 2025

mergify bot merged commit 1a90558 into ceph:master Jul 8, 2025
41 of 43 checks passed

implements: update modernc.org/cc pkg to v4 #1142

implements: update modernc.org/cc pkg to v4 #1142

Uh oh!

Conversation

Nikhil-Ladha commented Jun 18, 2025

Checklist

Uh oh!

Nikhil-Ladha commented Jun 18, 2025

Uh oh!

Nikhil-Ladha commented Jun 18, 2025

Uh oh!

anoopcs9 commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nikhil-Ladha commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phlogistonjohn commented Jun 18, 2025

Uh oh!

anoopcs9 commented Jun 18, 2025

Uh oh!

Nikhil-Ladha commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

phlogistonjohn commented Jun 18, 2025

Uh oh!

Nikhil-Ladha commented Jun 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nikhil-Ladha commented Jun 23, 2025

Uh oh!

anoopcs9 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anoopcs9 commented Jun 25, 2025

Uh oh!

Nikhil-Ladha commented Jun 25, 2025

Uh oh!

anoopcs9 commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nikhil-Ladha commented Jun 25, 2025

Uh oh!

anoopcs9 commented Jun 25, 2025

Uh oh!

Nikhil-Ladha commented Jun 26, 2025

Uh oh!

anoopcs9 left a comment

Choose a reason for hiding this comment

Uh oh!

Nikhil-Ladha commented Jul 7, 2025

Uh oh!

ansiwen left a comment

Choose a reason for hiding this comment

Uh oh!

anoopcs9 commented Jul 8, 2025

Uh oh!

mergify bot commented Jul 8, 2025

✅ Branch has been successfully rebased

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anoopcs9 commented Jun 18, 2025 •

edited

Loading

Nikhil-Ladha commented Jun 18, 2025 •

edited

Loading

Nikhil-Ladha commented Jun 18, 2025 •

edited

Loading

anoopcs9 commented Jun 25, 2025 •

edited

Loading