go proposal: cmd/vet: add a new analyzer to check for incorrect slice length initialization

Proposal Details

The following code exists in many projects, and developers actually want [0 1 2], but due to the initialization error of slice, the final result is [0 0 0 0 1 2]

package main

import "fmt"

func main() {

	sli := make([]int, 3)

	for i := range 3 {
		sli = append(sli, i)
	}

	fmt.Println(sli) // the result is [0 0 0 0 1 2]

}

The online demo: https://go.dev/play/p/q1BcVCmvidW

Over the past few months, I have conducted extensive research and analysis, and also submitted pull requests to fix issues in many well-known Go projects such as prometheus, zap, vitess. Below are some pull requests submitted by me and others related to this problem.

Due to limitations in search skills and time, I only checked records of such issues in the past few months. The history of more such issues has not been traced back. But I think it's already enough

I would like to propose adding a new analyzer to go vet that can detect such situations, thereby avoiding these issues in the future.

Now I have already completed an initial version of the code, and if the proposal is approved, I would be happy to refine it and add the necessary test cases.

The merged pr:

https://github.com/prometheus/prometheus/pull/14702#issuecomment-2302569768

https://github.com/uber-go/zap/pull/1461

https://github.com/uber/cadence/pull/6293

https://github.com/prometheus/prometheus/pull/15026

https://github.com/vitessio/vitess/pull/16674

https://github.com/kedacore/keda/pull/6179

https://github.com/external-secrets/external-secrets/pull/3964

https://github.com/brianvoe/gofakeit/pull/365

https://github.com/fission/fission/pull/3018

https://github.com/DataDog/datadog-agent/pull/29744

https://github.com/superseriousbusiness/gotosocial/pull/3382

https://github.com/ccfos/nightingale/pull/2169

https://github.com/gookit/color/pull/97

https://github.com/vdaas/vald/pull/2672

https://github.com/supabase/auth/pull/1788

https://github.com/pufferpanel/pufferpanel/pull/1367

https://github.com/juju/juju/pull/18176

https://github.com/go-spatial/tegola/commit/0f3131fbe428368012757aecfe38f57ec1844c6a

https://github.com/lxc/incus/pull/1285

https://github.com/yunionio/cloudpods/pull/21346

https://github.com/taubyte/tau/pull/253

https://github.com/fleetdm/fleet/pull/22608

https://github.com/antrea-io/antrea/pull/6715

https://github.com/tdewolff/canvas/pull/315

https://github.com/Consensys/gnark/pull/1288

https://github.com/superfly/flyctl/pull/3982

https://github.com/bazelbuild/rules_go/pull/4133

https://github.com/zitadel/oidc/pull/658

https://github.com/jhump/protoreflect/pull/629

https://github.com/apache/rocketmq-client-go/pull/1171

https://github.com/edgexfoundry/edgex-go/pull/4938

https://github.com/dolthub/doltgresql/pull/812

https://github.com/apache/trafficcontrol/pull/8091

https://github.com/pingcap/tidb-operator/pull/5755

https://github.com/botlabs-gg/yagpdb/pull/1734

https://github.com/Altinity/clickhouse-backup/pull/1019

https://github.com/openshift/installer/pull/9072

https://github.com/GoogleCloudPlatform/magic-modules/pull/11919

https://github.com/openmeterio/openmeter/pull/1615

https://github.com/target/goalert/pull/4090

https://github.com/kubeovn/kube-ovn/pull/4579

https://github.com/syyongx/php2go/pull/49

https://github.com/fluid-cloudnative/fluid/pull/4335

https://github.com/akuity/kargo/pull/2648

https://github.com/kubernetes/kubernetes/pull/127785

https://github.com/apache/dubbo-go/pull/2734

https://github.com/letsencrypt/boulder/pull/7725

https://github.com/cortexproject/cortex/pull/6237

https://github.com/kubeedge/kubeedge/pull/5895

https://github.com/grafana/mimir/pull/9449

https://github.com/rocboss/paopao-ce/pull/581

https://github.com/authelia/authelia/pull/7720

https://github.com/cilium/cilium/pull/35164

https://github.com/git-lfs/git-lfs/pull/5874

https://github.com/hashicorp/nomad/pull/24109/files

https://github.com/cosmos/ibc-go/pull/6444

https://github.com/minio/minio/pull/19567

https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6897

https://github.com/hyperledger/fabric/pull/4956

https://github.com/grafana/pyroscope/pull/3600

https://github.com/cosmos/cosmos-sdk/pull/21494#pullrequestreview-2274005038

https://github.com/anchore/grype/pull/2133

https://github.com/ethereum-optimism/optimism/pull/11542/files

https://github.com/libp2p/go-libp2p/pull/2938/files

https://github.com/stashapp/stash/pull/5327

https://github.com/trufflesecurity/trufflehog/pull/3293

https://github.com/c9s/bbgo/pull/1724#issuecomment-2322959753

https://github.com/cosmos/cosmos-sdk/pull/22006

https://github.com/FerretDB/FerretDB/pull/4598

https://github.com/dagger/dagger/pull/8612

https://github.com/letsencrypt/boulder/pull/7731

https://github.com/Layr-Labs/eigenda/pull/767

https://github.com/wal-g/wal-g/pull/1800

https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7161

https://github.com/harmony-one/harmony/pull/4767

https://github.com/stackrox/stackrox/pull/13028

ane more in review process.

Oct 14 '24 10:10 cuishuang

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

Oct 14 '24 10:10 gabyhelp

Change https://go.dev/cl/619915 mentions this issue: go/analysis: add a new analyzer to check for incorrect slice length initialization

Oct 14 '24 10:10 gopherbot

Change https://go.dev/cl/619935 mentions this issue: cmd: add a new analyzer to check for incorrect slice length initialization

Oct 14 '24 10:10 gopherbot

Change https://go.dev/cl/618875 mentions this issue: runtime: fix slice init length

Oct 14 '24 13:10 gopherbot

What is the proposed criteria to decide when vet should issue a report? It is hard to decide whether to accept a vet proposal without this being specified. We need to assess the likely false positive and negatives rates of the criteria. Thanks.

(You can try to turn the included CL into natural language.)

Oct 14 '24 16:10 timothy-king

I'm surprised that there are so many misuses of the append function.

Oct 14 '24 17:10 zigo101

Guessing from logic of the CL I just reviewed, the criterion is:

Vet should report a diagnostic when appending to a slice s if there exists an assignment s = make(T, n) where n is not a constant zero and there are no expressions of the form s[i] or copy(s, ...).

Oct 14 '24 17:10 adonovan

What is the proposed criteria to decide when vet should issue a report? It is hard to decide whether to accept a vet proposal without this being specified. We need to assess the likely false positive and negatives rates of the criteria. Thanks.

(You can try to turn the included CL into natural language.)

Yes, this is the issue I initially wanted to discuss on the CL page. Thanks to @adonovan for the detailed summary.

Additionally, through extensive practice, I have encountered several challenges and gained some insights: How do we balance false positives and omissions? Should we aim to comprehensively identify similar issues and accept a high rate of false positives, or should we focus on precision, which might lead to some omissions? This balance certainly warrants further discussion.

Specifically regarding the code, I’ve noticed that when slice are processed with binary.LittleEndian.PutUint16, they often lead to false positives. Moreover, if a slice is initialized in function A but only appended to after several layers of calls in function Z, should we address this situation? Or should we only consider initialization and appending that occur within the same method?

Last year, with your guidance, I added the "appends" analyzer to go vet, so I have some experience in this area.

However, it’s clear that this situation is more complex than last year, and there are more details to discuss. I look forward to your suggestions.

Thank you all!

Oct 15 '24 03:10 cuishuang

I'm surprised that there are so many misuses of the append function.

Yes, if using sli[i] for assignment, there is no such problem, but many people like me are used to using append. So I think it can also be considered as an initialization issue with the slice.

Based on my limited investigation and analysis of high star libraries related to the Go ecosystem, it seems worthwhile to add such an analyzer.

Oct 15 '24 03:10 cuishuang

I took a look at several of these reports. Most looks good. Some things to discuss. Here are my notes.

These all look like good reports to me. I think we can have stricter criteria and still report these. They are very similar.

https://github.com/prometheus/prometheus/pull/14702/files https://github.com/uber/cadence/pull/6293/files https://github.com/vitessio/vitess/pull/16674/files https://github.com/kedacore/keda/pull/6179/files https://github.com/brianvoe/gofakeit/pull/365/files https://github.com/fission/fission/pull/3018/files https://github.com/DataDog/datadog-agent/pull/29744/files https://github.com/superseriousbusiness/gotosocial/pull/3382/files https://github.com/ccfos/nightingale/pull/2169/files https://github.com/gookit/color/pull/97/files https://github.com/supabase/auth/pull/1788/files https://github.com/pufferpanel/pufferpanel/pull/1367/files https://github.com/juju/juju/pull/18176/files https://github.com/go-spatial/tegola/commit/0f3131fbe428368012757aecfe38f57ec1844c6a https://github.com/lxc/incus/pull/1285/files https://github.com/yunionio/cloudpods/pull/21346/files https://github.com/taubyte/tau/pull/253/files https://github.com/fleetdm/fleet/pull/22608/files https://github.com/antrea-io/antrea/pull/6715/files https://github.com/tdewolff/canvas/pull/315/files https://github.com/Consensys/gnark/pull/1288/files https://github.com/superfly/flyctl/pull/3982/files https://github.com/bazel-contrib/rules_go/pull/4133 https://github.com/zitadel/oidc/pull/658/files https://github.com/jhump/protoreflect/pull/629/files https://github.com/apache/rocketmq-client-go/pull/1171/files https://github.com/edgexfoundry/edgex-go/pull/4938/files https://github.com/apache/trafficcontrol/pull/8091/files https://github.com/pingcap/tidb-operator/pull/5755/files https://github.com/botlabs-gg/yagpdb/pull/1734/files https://github.com/Altinity/clickhouse-backup/pull/1019/files https://github.com/openshift/installer/pull/9072/files https://github.com/cortexproject/cortex/pull/6237/files https://github.com/letsencrypt/boulder/pull/7725/files https://github.com/kubernetes/kubernetes/pull/127785/files https://github.com/fluid-cloudnative/fluid/pull/4335/files https://github.com/syyongx/php2go/pull/49/files https://github.com/kubeovn/kube-ovn/pull/4579/files https://github.com/target/goalert/pull/4090/files https://github.com/openmeterio/openmeter/pull/1615/files https://github.com/openmeterio/openmeter/pull/1615/files https://github.com/cuishuang/vald/blob/2bb30dac79f0d9afdb0f809f0ca5b8eef0e62d75/internal/net/grpc/client.go https://github.com/GoogleCloudPlatform/magic-modules/pull/11919/files

This also looks like a good report, but had a more interesting fix:

https://github.com/akuity/kargo/pull/2648/files

These I am not as sure about. It would not be terrible to report these, but it would also not be too bad to not report them. Some of these should be rewritten as literals. Could we suggest alternative rewrites from the Analyzer?

https://github.com/dolthub/doltgresql/pull/812/files https://github.com/apache/dubbo-go/pull/2734/files

The proposed criteria would only have reported TestAzureKeyVaultSecretManagerGetAllSecrets in:

https://github.com/external-secrets/external-secrets/pull/3964/files This is a good example for why we need to exclude 0.

We should probably not report either of these from vet. This suggests to me we need to exclude slice expressions too.

https://github.com/prometheus/prometheus/pull/15026/files https://github.com/uber-go/zap/pull/1461/files

I am still not exactly sure I have a clear theory of what we are reporting yet. But the heuristics seem to be hitting on something promising. Indexing and slicing is out. range is fine. len is fine. Passing the slice as an argument or a variadic expansion... is also fine. Storing it in a field has been fine in these examples so far. I am not sure what this adds up to.

Another point, it would be good for us to have negative examples too. Making an all zero value slice needs to not be reported. And we should have guidance on how to create {0,0,0,1,2,3}. The proposal is going to flag a correct way to create this value. What is the 'right' way to do this? Do we need to give folks a clear escape hatch?

FWIW I think the proposal should be phrased 'as what to report' rather than 'add a new analyzer'. This could plausibly go into the "appends" analyzer.

Oct 17 '24 03:10 timothy-king

I took a look at several of these reports. Most looks good. Some things to discuss. Here are my notes.

These all look like good reports to me. I think we can have stricter criteria and still report these. They are very similar.

https://github.com/prometheus/prometheus/pull/14702/files https://github.com/uber/cadence/pull/6293/files https://github.com/vitessio/vitess/pull/16674/files https://github.com/kedacore/keda/pull/6179/files https://github.com/brianvoe/gofakeit/pull/365/files https://github.com/fission/fission/pull/3018/files https://github.com/DataDog/datadog-agent/pull/29744/files https://github.com/superseriousbusiness/gotosocial/pull/3382/files https://github.com/ccfos/nightingale/pull/2169/files https://github.com/gookit/color/pull/97/files https://github.com/supabase/auth/pull/1788/files https://github.com/pufferpanel/pufferpanel/pull/1367/files https://github.com/juju/juju/pull/18176/files go-spatial/tegola@0f3131f https://github.com/lxc/incus/pull/1285/files https://github.com/yunionio/cloudpods/pull/21346/files https://github.com/taubyte/tau/pull/253/files https://github.com/fleetdm/fleet/pull/22608/files https://github.com/antrea-io/antrea/pull/6715/files https://github.com/tdewolff/canvas/pull/315/files https://github.com/Consensys/gnark/pull/1288/files https://github.com/superfly/flyctl/pull/3982/files bazel-contrib/rules_go#4133 https://github.com/zitadel/oidc/pull/658/files https://github.com/jhump/protoreflect/pull/629/files https://github.com/apache/rocketmq-client-go/pull/1171/files https://github.com/edgexfoundry/edgex-go/pull/4938/files https://github.com/apache/trafficcontrol/pull/8091/files https://github.com/pingcap/tidb-operator/pull/5755/files https://github.com/botlabs-gg/yagpdb/pull/1734/files https://github.com/Altinity/clickhouse-backup/pull/1019/files https://github.com/openshift/installer/pull/9072/files https://github.com/cortexproject/cortex/pull/6237/files https://github.com/letsencrypt/boulder/pull/7725/files https://github.com/kubernetes/kubernetes/pull/127785/files https://github.com/fluid-cloudnative/fluid/pull/4335/files https://github.com/syyongx/php2go/pull/49/files https://github.com/kubeovn/kube-ovn/pull/4579/files https://github.com/target/goalert/pull/4090/files https://github.com/openmeterio/openmeter/pull/1615/files https://github.com/openmeterio/openmeter/pull/1615/files https://github.com/cuishuang/vald/blob/2bb30dac79f0d9afdb0f809f0ca5b8eef0e62d75/internal/net/grpc/client.go https://github.com/GoogleCloudPlatform/magic-modules/pull/11919/files

This also looks like a good report, but had a more interesting fix:

https://github.com/akuity/kargo/pull/2648/files

These I am not as sure about. It would not be terrible to report these, but it would also not be too bad to not report them. Some of these should be rewritten as literals. Could we suggest alternative rewrites from the Analyzer?

https://github.com/dolthub/doltgresql/pull/812/files https://github.com/apache/dubbo-go/pull/2734/files

The proposed criteria would only have reported TestAzureKeyVaultSecretManagerGetAllSecrets in:

https://github.com/external-secrets/external-secrets/pull/3964/files This is a good example for why we need to exclude 0.

We should probably not report either of these from vet. This suggests to me we need to exclude slice expressions too.

https://github.com/prometheus/prometheus/pull/15026/files https://github.com/uber-go/zap/pull/1461/files

I am still not exactly sure I have a clear theory of what we are reporting yet. But the heuristics seem to be hitting on something promising. Indexing and slicing is out. range is fine. len is fine. Passing the slice as an argument or a variadic expansion... is also fine. As is storing it in a field has been fine in these examples. I am not sure what this adds up to.

Another point, it would be good for us to have negative examples too. Making an all zero value slice needs to be fine. And we should have guidance on how to create {0,0,0,1,2,3}. The proposal is going to flag a correct way to create this value. What is the 'right' way to do this? Do we need to give folks a clear escape hatch?

FWIW I think the proposal should be phrased 'as what to report' rather than 'add a new analyzer'. This could plausibly go into the "appends" analyzer.

Thanks for your reply and review. I have had a lot of discussions with @adonovan about this on Gerrit.

Under adonovan's suggestion and guidance, I have now added the SuggestedFix feature.

Regarding this fix in https://github.com/akuity/kargo/pull/2648/files, it highlights the difference between using sli[i] and append, but the end result is the same. I might prefer using append, but all the way is ok.

Regarding whether to extend the existing appends analyzer or to add a new one, here are my thoughts: https://go-review.googlesource.com/c/tools/+/619915/comments/0e76d631_e6c99d92

Regarding other questions, such as "What is the 'right' way to do this? Do we need to give folks a clear escape hatch?" I'm also not sure, so perhaps I should listen to other‘s suggestions.

Overall, at least for now, adding such checks is valuable, and there are many details that can still be discussed and improved.

I will pay attention to and follow the final advice of the Go team. Thank you again!

Oct 17 '24 04:10 cuishuang

Change https://go.dev/cl/620316 mentions this issue: all: register the sliceinit analyzer in a few places and update gopls/go.mod

Oct 18 '24 07:10 gopherbot

Do we need to give folks a clear escape hatch?

To answer my own question, the escape hatch is to write s := make([]T, n, n) instead of s := make([]T, n) when one intends to initialize the length to n as well. Obvious in retrospect.

It seems okay to encourage this as an escape hatch as the checker is trying to find cases where slice's (len,cap) is initialized to (n,n) when the checker 'thinks' (0,n) is almost certainly intended.

Oct 22 '24 08:10 timothy-king

After communicating with the two maintainers and receiving very good suggestions, we decided not to add a new analyzer, but to add these features based on the previous appends analyzer.

Does this still need to be proposed?

Oct 23 '24 15:10 cuishuang

Yes, major feature changes to analyzers included in vet do need proposals.

Oct 23 '24 17:10 adonovan

Yes, major feature changes to analyzers included in vet do need proposals.

Thanks. I have moved the code to the appends directory. Some comments may need to be optimized. Please review it when you have time.

Oct 23 '24 17:10 cuishuang

I ran the new analyzer (at patch set 18 of CL 619915) over about 15,000 modules, and got 1225 new findings. Here are a random 50.

Inspecting the first 10, at least 7 are false positives, most relating to functions like binary.BigEndian.PutUint32 that fill a fixed-size buffer that is later appended. It does turn up some real bugs though. More work is needed to reduce the false-positive rate beneath our target of 5%.

Oct 23 '24 19:10 adonovan

Thanks Alan. Those FPs in the first 10 or so examples are clear cases of wanting to generate {0,0,0,0,1,2,3,4}. We will need a more specific criteria.

Oct 24 '24 02:10 timothy-king

Additionally, through extensive practice, I have encountered several challenges and gained some insights: How do we balance false positives and omissions? Should we aim to comprehensively identify similar issues and accept a high rate of false positives, or should we focus on precision, which might lead to some omissions? This balance certainly warrants further discussion.

Specifically regarding the code, I’ve noticed that when slice are processed with binary.LittleEndian.PutUint16, they often lead to false positives. Moreover, if a slice is initialized in function A but only appended to after several layers of calls in function Z, should we address this situation? Or should we only consider initialization and appending that occur within the same method?

Thanks @adonovan ! In my practice, the main false positives also come from binary.LittleEndian.PutUint16 related methods.

I agree with what @timothy-king said, maybe we need to add some more standards on the basis of the above three standards to reduce the false positive rate. ( But I am surprised why there are so many cases where the original intention is to get {0,0,0,0,1,2,3,4} )

Do the two maintainers and community developers have any suggestions?

My current idea is that if sli is called by binary.LittleEndian.PutUint16 related methods, it is ignored, just like it has been called by copy or sli[i]

Oct 24 '24 03:10 cuishuang

My current idea is that if sli is called by binary.LittleEndian.PutUint16 related methods, it is ignored, just like it has been called by copy or sli[i]

We can try to list all of the stdlib functions that might be used to intentionally append to a slice, but my intuition is that this will not be sufficient. We have to also consider user appending functions.

My recommendation is to go back to the original examples and to try to tease out why we think those examples are bugs more precisely. We can re-implement once we better understand this.

Oct 24 '24 05:10 timothy-king

We can try to list all of the stdlib functions that might be used to intentionally append to a slice, but my intuition is that this will not be sufficient. We have to also consider user appending functions.

My recommendation is to go back to the original examples and to try to tease out why we think those examples are bugs more precisely. We can re-implement once we better understand this.

Okay, I will list out the methods in the standard library that trigger false positives, such as binary.LittleEndian.PutUint16. Regarding the "original examples," I want to confirm whether you are referring to the initial PRs I listed that were merged, or the results provided by Alan. I believe it should be the latter. We just need to analyze the common situations that lead to false positives, and by modifying the code accordingly, we can significantly reduce the false positive rate to an acceptable level.
Most of the false positives are caused by slices of type []byte. Ignoring checks for this type of slice could immediately reduce the false positive rate, but this approach seems a bit aggressive.

Oct 24 '24 15:10 cuishuang

At present, through the analysis of the corpus information provided by Alan and my previous analysis of the go project on GitHub, the following situations occur when calling a method of the standard library and a false positive occurs:

        binary.LittleEndian.PutUint16(): https://github.com/cloudflare/circl/blob/main/abe/cpabe/tkn20/internal/tkn/policy.go#L84:8
	binary.LittleEndian.PutUint32(): https://github.com/nknorg/nkn/blob/master/chain/store/store.go#L480:12
	binary.LittleEndian.PutUint64(): https://github.com/ethersphere/bee/blob/master/pkg/storage/storagetest/storage.go#L101:8   

	binary.LittleEndian.Uint16()
	binary.LittleEndian.Uint32()
	binary.LittleEndian.Uint64()

	binary.LittleEndian.AppendUint16()
	binary.LittleEndian.AppendUint32()
	binary.LittleEndian.AppendUint64()

	binary.BigEndian.PutUint16():  https://github.com/XZB-1248/Spark/blob/master/client/service/desktop/desktop.go#L233:8
	binary.BigEndian.PutUint32()
	binary.BigEndian.PutUint64():  https://github.com/bnb-chain/bsc/blob/master/core/vm/contracts_lightclient.go#L105:11

	binary.BigEndian.Uint16()
	binary.BigEndian.Uint32()
	binary.BigEndian.Uint64()

	binary.BigEndian.AppendUint16()
	binary.BigEndian.AppendUint32()
	binary.BigEndian.AppendUint64()

I have submitted a new patch to exclude the above situation.

But if this method is not in the standard library but is user-defined, should it still be considered?

(Perhaps based on practical experience, the false positive rate after excluding binary.BigEndian is acceptable)

Oct 24 '24 15:10 cuishuang

Are you able to estimate the the false positive rate when these cases are excluded? The file attached above contains the complete list of 1225 findings. Can you sample a couple of dozen?

Oct 24 '24 16:10 adonovan

Are you able to estimate the the false positive rate when these cases are excluded? The file attached above contains the complete list of 1225 findings. Can you sample a couple of dozen?

I conducted a detailed analysis, and out of 1,225 cases, approximately 375 of them are related to the usage of the standard library's methods involving binary.BigEndian or binary.LittleEndian to process slices. (I simply wrote a program to detect whether the page contained the keywords binary.BigEndian or binary.LittleEndian. I didn’t further check if slices were actually passed as parameters to these methods, but the margin of error is very small. ) For the remaining 850 cases, I analyzed the first 100.

(And also randomly selected 20 from the remaining 775.)

The detailed result: https://github.com/cuishuang/appends-analysis Consider the first 100 as an example: 24 of them were clearly false positives. Among these, 22 were due to calls to the standard library functions rand.Read(sli) (cases 6, 7, 8, 9, 10, 11, 12, 15, 27, 29, 30, 31, 35, 36, 66), io.ReadFull(rand.Reader, sli) (cases 2, 18, 19, 38), cipher.XORKeyStream (cases 77, 78), and binary.PutUvarint (case 86), which we can easily avoid reporting . In case 87, the usage of unicode.ToUpper does not seem to be a false positive in my opinion. Additionally: In case 4:

out := make([]rune, len(slice))
out = out[:0]

This is essentially equivalent to out := make([]rune, 0, len(slice)). Perhaps it is necessary to exclude operations like sli[i:j]. For cases 5, 62, and 63, there are copy operations, but we still flagged these for detection. We need to investigate why they were not ignored. There are about a dozen cases still in the "todo" state, mainly because they involve calls to user-defined methods, such as:

Case 1: runes.Convert from "9fans.net/go/cmd/acme/internal/runes"
Case 16: util.ReadAtMost
Case 60: fee.Amount.BigInt().FillBytes(sli)
Cases 79, 80: Wrap
Cases 93, 97: utils.NewBitWriter(utils.NewWriterAtBuffer(sli) Some of these might not be false positives, It's actually a bug that needs to be fixed. such as:
Cases 48, 53, 55 Aside from these, the rest are fairly standard errors and should be reported.

In summary, when the slice type is []byte, and it's often used in cryptographic contexts, the use of make([]byte, n) is usually exactly what the developer intended.

If we handle the listed methods from the standard library while ignoring user-defined methods, I believe the false positive rate can be reduced to around 5%.

Oct 25 '24 13:10 cuishuang

I have put this proposal on Hold until we have a promising prototype implementation.

Nov 20 '24 17:11 adonovan

I have put this proposal on Hold until we have a promising prototype implementation.

OK, thanks. I'll keep an eye on its progress.

If we reach an agreement, I'll be happy to implement its code.

Nov 21 '24 09:11 cuishuang

If we reach an agreement, I'll be happy to implement its code.

The order is backwards: what this proposal needs to move forwards is a new idea, a prototype, an experiment, and some encouraging data; agreement will follow from that.

Nov 21 '24 13:11 adonovan

If we reach an agreement, I'll be happy to implement its code.

The order is backwards: what this proposal needs to move forwards is a new idea, a prototype, an experiment, and some encouraging data; agreement will follow from that.

OK, I understand.

My current idea is to add checks for special methods in the standard library (such as binary.BigEndian), or to exclude the []byte type more aggressively.

This is indeed a widespread problem, but it is not easy to reduce the false positive rate in implementation. Maybe there will be a better solution later.

Thanks for your patience guidance and review.

Nov 21 '24 14:11 cuishuang