跳到正文
DEV Community· Unicorn Developer·· 3 小時前AI 評分41

複製貼上出錯時：結構同構 Go 程式碼中的錯誤

When copy-paste goes wrong: Bugs in structurally isomorphic Go code

AI 導讀

PVS-Studio 的 Go 分析器以 V8033 診斷規則，透過簡化 AST 分組並比較結構相似的程式碼片段，偵測複製貼上造成的變數錯誤。這套方法不需 AI，而是採用啟發式演算法；診斷規則開發中，95% 的時間用於教導分析器識別看似可疑但實際正確的編程技巧。

正文

You might've done this many times: you found code in a project that seemed like a good fit for your task, copied it, replaced the variables or function calls with the ones you needed, and then got behavior you didn't expect when you ran the code. It's likely that you missed something while making those changes. This is exactly how the errors described in this article occur.

Introduction

PVS-Studio 8.00 was recently released, introducing several new major features, including the Go analyzer that comes with several new and cool diagnostic rules.

It includes a rule that detects one of the trickiest types of copy-paste bugs, the kind that can easily slip past a regular code review. Okay, now let's take a look at the code:

if process.ProcessName != nil && 
   (len(*process.ProcessName) == 0 ||
   len(*process.ProcessName) > common.NameFieldMaxLength) {

  return kit.CCError.CCErrorf(common.CCErrCommParamsInvalid,        
                              common.BKProcessNameField)
}
....
if process.FuncName != nil && 
   (len(*process.FuncName) == 0 ||
   len(*process.ProcessName) > common.NameFieldMaxLength) {

  return kit.CCError.CCErrorf(common.CCErrCommParamsInvalid, 
                              common.BKFuncName)
}

You've probably noticed that the expressions in both if statements consist of the same checks, but they aren't completely identical: the first one uses process.ProcessName, while the second one uses process.FuncName in the same positions. Or... does it?

The last checks suspiciously break this pattern: they're exactly the same, and both use the process.ProcessName field. Based on the logic the code has consistently followed so far, the check in the second if should have used a different field, process.FuncName.

Since such typos can easily slip past manual review, the best solution is to delegate the task of catching them to a static analyzer. But how can we teach a program to compare similar yet distinct code fragments and use those comparisons to detect bugs the way humans do? Is AI necessary for that? In the rest of this article, we'll explore these questions by looking at how the diagnostic rule works in PVS-Studio for Go.

How does the analyzer find these bugs?

Modern AI models can indeed detect such bugs. However, integrating artificial intelligence into a static analyzer for such a purpose is too expensive and resource-intensive.

Fortunately, we don't need AI here. We can handle the task with a relatively simple heuristic algorithm. We'll take a closer look at how it works later, but first, let's go over a few basic concepts from static code analysis:

  • Statements are the elements of code that form its skeleton and describe the execution flow (if, switch, for, and {...} blocks).
  • Expressions are code elements that manage data (function calls, variables, and operators).
  • Nodes is a general term for statements, expressions, and other types of code elements.
  • Abstract Syntax Tree (AST) represents code as a hierarchical structure where some elements, like folders in a file system, contain other elements. An AST contains the same information you can get from the code itself, but its structured format makes it better suited for program analysis.

If you're interested in delving deeper into static analysis, we have the perfect article for you.

Well, now that we have enough context, we're ready to face the boss algorithm. Here's how the V8033 diagnostic rule in PVS-Studio's Go analyzer works:

  1. Within a function, the analyzer takes individual statements and groups of statements, separated by blank lines (hereinafter referred to as code fragments) and builds a simplified AST for them. Unlike a regular AST, it contains only the information needed to describe the code structure and omits specific names of variables, functions, and other semantic information.
  2. The fragments are grouped by their simplified AST. This is how fragments with similar structure are identified, even though they may still differ in their details (for example, in the variables they use).
  3. Within each group, the fragments are paired and compared with one another. Let's look at how the algorithm works for a single pair. For convenience, let's call the two fragments in one pair A and B.
  4. The analyzer identifies the variables that appear in both A and B (common variables) as well as the variables that appear in only one of the fragments (specific variables).
  5. Let's assume that A is higher in the code than B. If one of the common variables occurs less frequently in B than in A, but still at the same relative positions, the uses of that variable in B are treated as potential typos.
  6. Next, a replacement candidate is sought among the B variables. This variable must be used in the same relative position as the previously considered general variable in A, but not in B.
  7. If the analyzer finds such a candidate, it performs a series of final checks to determine whether the variables are interchangeable. If all the checks pass, the developer receives warnings about possible typos in the code.

Of course, this is just a conceptual outline. A significant amount of important logic remains behind the scenes, especially the logic that filters out false positives. Without it, the diagnostic rule would be far less useful.

It is only 5% of the task to create the diagnostic. It's not hard for the analyzer to complain about suspicious code, it's much harder to not complain about the correct code. We spend 95% of our time "teaching" the analyzer to detect various programming techniques, which may seem suspicious for the diagnostic, but in reality they are correct.

From the article "How PVS-Studio does the bug search: methods and technologies"

Now that we've taken a quick look at how the analyzer works, let's exercise in looking for similar errors on our own.

Find an error

I invite you to challenge yourselves by finding errors in this code, which was taken from real open-source projects. The closer you get to the end, the harder the examples become.

Are you up for the challenge? Let's get started, then!

Yao Agents

Yao Agents is an open-source platform for creating and running autonomous AI agents. Its key feature is that the agents run directly on users' devices, while all operations are centralized and controlled via a single interface.

We found this code in the v1.0.0-rc15 release:

func Generate(option Option) (string, string) {
  if option.Width == 0 {
    option.Width = 240
  }

  if option.Height == 0 {
    option.Width = 80
  }

  if option.Length == 0 {
    option.Length = 6
  }

  if option.Lang == "" {
    option.Lang = "zh"
  }
  ....
}

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "option.Height" variable should be used instead of the "option.Width". captcha.go 48

CDS

CDS is an enterprise-grade platform for automating DevOps processes, continuous integration (CI), and continuous delivery (CD).

We found this code in the 0.54.1 release:

if conf.GraylogFieldCDSServiceName != "" {
  extra["CDSName"] = conf.GraylogFieldCDSServiceName
}
if conf.GraylogFieldCDSServiceName != "" {
  extra["CDSService"] = conf.GraylogFieldCDSServiceType
}
if conf.GraylogFieldCDSVersion != "" {
  extra["CDSVersion"] = conf.GraylogFieldCDSVersion
}
if conf.GraylogFieldCDSOS != "" {
  extra["CDSOS"] = conf.GraylogFieldCDSOS
}
if conf.GraylogFieldCDSArch != "" {
  extra["CDSArch"] = conf.GraylogFieldCDSArch
}

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "conf.GraylogFieldCDSServiceType" variable should be used instead of the "conf.GraylogFieldCDSServiceName". log.go 107

BFE

BFE is a high-performance, application-level load balancer and reverse proxy server.

We found this code in the 1.8.4 release:

if cfg.Basic.WasmPluginPath == "" {
  ....
  cfg.Basic.WasmPluginPath = "mod_wasm"
}
cfg.Basic.WasmPluginPath = bfe_util.ConfPathProc(....)

if cfg.Basic.DataPath == "" {
  ....
  cfg.Basic.WasmPluginPath = "mod_wasm/wasm.data"
}
cfg.Basic.DataPath = bfe_util.ConfPathProc(cfg.Basic.DataPath, 
                                           confRoot)

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "cfg.Basic.DataPath" variable should be used instead of the "cfg.Basic.WasmPluginPath". conf_mod_wasmplugin.go 63

Hertz

Hertz is a high-performance HTTP framework written in Go by ByteDance (the creators of TikTok), designed for building high-load microservices. It combined the concepts of popular Go frameworks (Gin, FastHTTP) with ByteDance's internal requirements.

We found this code in the 0.10.6 release:

if goOptMapAlias[sb.Scope.GetOptions().GetGoPackage()] != "" {
  reqName = goOptMapAlias[sb.Scope.GetOptions().GetGoPackage()] + 
            "." + inputGoType.GoIdent.GoName
}
if goOptMapAlias[sb.Scope.GetOptions().GetGoPackage()] != "" {
  respName = goOptMapAlias[st.Scope.GetOptions().GetGoPackage()] + 
             "." + outputGoType.GoIdent.GoName
}

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "st" variable should be used instead of the "sb". ast.go 233

Nuclei

Nuclei is a modern, high-performance vulnerability scanner that uses simple YAML-based templates. It enables users to create their own vulnerability detection scenarios that simulate real-world conditions, which eliminates false positives.

We found this code in the 3.11.1 release:

for _, tag := range config.ExcludeTags {
  for _, val := range splitCommaTrim(tag) {
    if _, ok := filter.block[val]; !ok {
      filter.block[val] = struct{}{}
    }
  }
}
for _, tag := range config.Severities {
  if _, ok := filter.severities[tag]; !ok {
    filter.severities[tag] = struct{}{}
  }
}
....
for _, tag := range config.ExcludeProtocols {
  if _, ok := filter.excludeTypes[tag]; !ok {
    filter.excludeTypes[tag] = struct{}{}
  }
}
for _, id := range config.ExcludeIds {
  for _, val := range splitCommaTrim(id) {
    if _, ok := filter.block[val]; !ok {
      filter.excludeIds[val] = struct{}{}
    }
  }
}

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "filter.excludeIds" variable should be used instead of the "filter.block". tag_filter.go 434

LakeFS

LakeFS is an open-source data version control system for managing data stored in an object storage system. It enables Git-like operations like branching, committing, merging, and rolling back for large-scale data, stored in systems including Amazon S3, Azure Blob Storage, and Google Cloud Storage, as well as on other S3-compatible object storage platforms.

We found this code in the 1.86.0 release:

usersKey := model.GroupUserPath(groupID, "")

it, err := kv.NewSecondaryIterator(....)

if err != nil {
  return err
}
defer it.Close()
for it.Next() {
  entry := it.Entry()
  user := entry.Value.(*model.UserData)
  ....
}
if err = it.Err(); err != nil {
  return err
}

policiesKey := model.GroupPolicyPath(groupID, "")

itr, err := kv.NewSecondaryIterator(....)

if err != nil {
  return err
}
defer it.Close()
for itr.Next() {
  entry := itr.Entry()
  policy := entry.Value.(*model.PolicyData)
  ....
}
if err = itr.Err(); err != nil {
  return err
}

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "itr" variable should be used instead of the "it". service.go 476

Vault

Vault is an open-source (BSL) tool for centralized secret management, data encryption, and for the protection of confidential information in cloud and hybrid infrastructures.

We found this code in the 2.0.4 release:

for name, crl := range h.CRLs {
  ....
  ratio := time.Duration(
    (int64(total) * int64(h.CRLExpiryPercentage)) / int64(100)
  )
  if used >= ratio {
    expWhen := crl.ThisUpdate.Add(ratio)
    ret.Status = ResultCritical
    ret.Message = fmt.Sprintf(
      ...., 
      crl.NextUpdate.Format("2006-01-02"), 
      h.CRLExpiryPercentage,   
      crl.ThisUpdate.Format("2006-01-02"), 
      expWhen.Format("2006-01-02")
    )
    ....
  }

  results = append(results, &ret)
}

for name, crl := range h.DeltaCRLs {
  ....
  ratio := time.Duration(
    (int64(total) * int64(h.DeltaCRLExpiryPercentage)) / int64(100)
  )
  if used >= ratio {
    expWhen := crl.ThisUpdate.Add(ratio)
    ret.Status = ResultCritical
    ret.Message = fmt.Sprintf(
      ...., 
      crl.NextUpdate.Format("2006-01-02"),
      h.CRLExpiryPercentage, 
      crl.ThisUpdate.Format("2006-01-02"), 
      expWhen.Format("2006-01-02")
    )
    ....
  }

  results = append(results, &ret)
}

The analyzer's hint
V8033 Two similar code fragments were found. The second fragment may contain a typo: the "h.DeltaCRLExpiryPercentage" variable should be used instead of the "h.CRLExpiryPercentage". pki_crl_validity_period.go 195

Conclusion

Did you spot all the errors without looking at the hints? If so, that's great! If not, don't worry—that's where a static code analyzer comes into play!

By the way, you can download it from our official website and try it for free on your own projects.

In addition to the Go analyzer, PVS-Studio 8.00 brought several other exciting features, including an analyzer for JavaScript and TypeScript. You can find more details about them in our article.

Happy coding!

來源：DEV Community · dev.to

#教學/實踐#編碼