One mistake no one has ever made. Right?

As early as your freshman year of college—or even earlier—you might have realized that writing arr[len(arr)] is a bad idea. Surely, no one could get that wrong, right? We thought the same thing, until we checked 1,000 projects.

A quick explanation of the error, just in case

Any Go developer knows that the built-in len() function takes a value and returns its length based on the value's type.

Let's check the documentation and take a closer look at a function signature:

func len(v Type) int

The built-in len function returns the length of v based on the type of the passed value:

• Array: the number of elements in v.
• Pointer to array: the number of elements in *v (even if v is nil).
• Slice, or map: the number of elements in v; if v is nil, len(v) is zero.
• String: the number of bytes in v.
• Channel: the number of elements queued (unread) in the channel buffer; if v is nil, len(v) is zero.

Look at this simple example:

func SimpleExample(arr []int) {
  _ = arr[len(arr)]
}

Collection indices always start at 0. So, for a slice of the n length, the valid indices range from 0 to n – 1. Then len(arr) returns n. As a result, accessing arr[len(arr)] always leads to a program panic.

What's V8031, and how did we discover it?

First, a quick note: in case you didn't know, we're developing PVS-Studio, a static analyzer for C, C++, C#, and Java code. The early access program for the new Go and JS/TS analyzers just ended. We're currently preparing them for the release, which is scheduled for August. Around the same time, the early access program for PVS-Studio Atlas—a team solution for managing code analysis results—will launch. Stay tuned for updates.

Now back to V8031. It's a diagnostic rule for the Go analyzer. We didn't expect it to deliver such good results when we were working on it. We even assumed it might not find anything. We thought, "What if this is a made-up error that can be spotted in any review or test?" From our experience, though, we knew there could be dozens of such errors that are easy to spot but often overlooked due to exhaustion, carelessness, or a lack of effort.

Implementing the diagnostic rule was easy: it's built entirely on AST and uses as little of our own code as possible beyond what Go's built-in analysis tools provide. Feel free to create your own diagnostic rules, by the way. You might want to start with our introductory article, "How to create your own Go static analyzer?"

The next step is testing. In our team, it consists of three stages:

1. Unit testing;
2. Running a diagnostic rule on our collection of GitHub projects;
3. Running a diagnostic rule on the 1,000 most popular Go projects (selected by number of stars).

Right now, we'll focus on the third stage. We were surprised by the results as soon as we got them. Let's take a look at them.

The results

In our articles, we usually describe errors and explain possible ways to fix them. We won't do that here since all the code snippets would have the same description as the one at the beginning of the article.

Project No. 1

bfe is a modern load balancer used by Baidu.

type BasicLitList []*BasicLit
....
func (b BasicLitList) End() token.Pos {
  return b[len(b)].End()
}

PVS-Studio warning: V8031 Using the call of the 'len' function as index will cause off-by-one error. Consider using 'len(b) - 1' instead. ast.go 138

Project No. 2

Ferret is a programmable data extraction and automation tool for developers.

func (vec *Vector) Push(value core.Value) *Vector {
  slice := vec.getCurrentSlice()
  slice[len(slice)] = value

  return vec
}

PVS-Studio warning: V8031 Using the call of the 'len' function as index will cause off-by-one error. Consider using 'len(slice) - 1' instead. vector.go 28

Project No. 3

fq is a tool, language, and set of decoders for working with binary and text formats.

var blockFns = map[uint64]func(d *decode.D, dc *decodeContext){
  ....
  blockTypeInterfaceDescription: func(d *decode.D, dc *decodeContext) {
    typ := d.FieldU16("link_type", format.LinkTypeMap)
    d.FieldU16("reserved")
    d.FieldU32("snap_len")
    d.FieldArray("options", ....)

  dc.interfaceTypes[len(dc.interfaceTypes)] = int(typ)
  },
  ....
}

PVS-Studio warning: V8031 Using the call of the 'len' function as index will cause off-by-one error. Consider using 'len(dc.interfaceTypes) - 1' instead. pcapng.go 234

Project No. 4

Incus is a modern container management system and virtual machine manager.

func qemuRawCfgOverride(conf []cfg.Section, confOverride string) (....) {
  ....
  content := &sectionContent{
    comment: sec.Comment,
    entries: entries,
  }

  indexedSection[len(indexedSection)] = content
  orderedSections = append(orderedSections, section{
    name:    sec.Name,
    content: content,
  })
  ....
}

PVS-Studio warning: V8031 Using the call of the 'len' function as index will cause off-by-one error. Consider using 'len(indexedSection) - 1' instead. driver_qemu_config_override.go 50

We spotted two such errors in this project:

func (f *heartbeatFixture) node() (*state.State, *cluster.Gateway, string) {
  ....
  f.gateways[len(f.gateways)] = gateway
  f.states[gateway] = state
  f.servers[gateway] = server

  return state, gateway, address
}

V8031 Using the call of the 'len' function as index will cause off-by-one error. Consider using 'len(f.gateways) - 1' instead. heartbeat_test.go 253

We've encountered some errors multiple times in other projects. The reason for this was that some Project 1 included some other Project 2 as a dependency. So, it's fair to say that the errors are piling up.

Let's summarize

We wanted to share these exciting results with you. Although unlikely, this error may lurk in your own project. Not long ago, we discussed another interesting error in the article Does the operator ^ represent exponentiation or exclusion?. And to ensure your project's code is clean, you can use PVS-Studio static analyzer.

Happy coding!