doctor: add validation runtime error recovery

Previously, doctor could crash when validating corrupted descriptors.
The validation code is usually cautious enough to not dereference nils
or the like but this isn't enforced in any way.

This commit protects the validation calls in doctor with panic recovery,
allowing doctor to continue and validate the remaining descriptors.

Release justification: non-production code change.
Release note: None

pkg/sql/doctor/doctor.go

@@ -140,7 +140,7 @@ func ExamineDescriptors(
 			continue
 		}
 
-		for _, err := range catalog.Validate(ctx, descGetter, catalog.ValidationLevelSelfAndCrossReferences, desc).Errors() {
+		for _, err := range validateSafely(ctx, descGetter, desc) {
 			problemsFound = true
 			fmt.Fprint(stdout, reportMsg(desc, "%s", err))
 		}
@@ -237,6 +237,23 @@ func ExamineDescriptors(
 	return !problemsFound, err
 }
 
+func validateSafely(
+	ctx context.Context, descGetter catalog.MapDescGetter, desc catalog.Descriptor,
+) (errs []error) {
+	defer func() {
+		if r := recover(); r != nil {
+			err, ok := r.(error)
+			if !ok {
+				err = errors.Newf("%v", r)
+			}
+			err = errors.WithAssertionFailure(errors.Wrap(err, "validation"))
+			errs = append(errs, err)
+		}
+	}()
+	errs = append(errs, catalog.Validate(ctx, descGetter, catalog.ValidationLevelSelfAndCrossReferences, desc).Errors()...)
+	return errs
+}
+
 // ExamineJobs runs a suite of consistency checks over the system.jobs table.
 func ExamineJobs(
 	ctx context.Context,

pkg/sql/doctor/doctor_test.go

@@ -245,6 +245,46 @@ func TestExamineDescriptors(t *testing.T) {
 			expected: `Examining 2 descriptors and 2 namespace entries...
   ParentID   3, ParentSchemaID  2: type "type" (1): referenced schema ID 2: descriptor not found
   ParentID   3, ParentSchemaID  2: type "type" (1): arrayTypeID 0 does not exist for "ENUM": referenced type ID 0: descriptor not found
+`,
+		},
+		{
+			descTable: doctor.DescriptorTable{
+				{
+					ID: 51,
+					DescBytes: toBytes(t, &descpb.Descriptor{Union: &descpb.Descriptor_Database{
+						Database: &descpb.DatabaseDescriptor{Name: "db", ID: 51},
+					}}),
+				},
+				{
+					ID: 52,
+					DescBytes: func() []byte {
+						// Skip `toBytes` to produce a descriptor with unset privileges field.
+						// The purpose of this is to produce a nil dereference during validation
+						// in order to test that doctor recovers from this.
+						//
+						// Note that it might be the case that validation aught to check that
+						// this field is not nil in the first place, in which case this test case
+						// will need to craft a corrupt descriptor serialization in a more
+						// creative way. Ideally validation code should never cause runtime errors
+						// but there's no way to guarantee that short of formally verifying it. We
+						// therefore have to consider the possibility of runtime errors (sadly) and
+						// doctor should absolutely make every possible effort to continue executing
+						// in the face of these, considering its main use case!
+						desc := &descpb.Descriptor{Union: &descpb.Descriptor_Type{
+							Type: &descpb.TypeDescriptor{Name: "type", ID: 52, ParentID: 51, ParentSchemaID: keys.PublicSchemaID},
+						}}
+						res, err := protoutil.Marshal(desc)
+						require.NoError(t, err)
+						return res
+					}(),
+				},
+			},
+			namespaceTable: doctor.NamespaceTable{
+				{NameInfo: descpb.NameInfo{Name: "db"}, ID: 51},
+				{NameInfo: descpb.NameInfo{ParentID: 51, ParentSchemaID: keys.PublicSchemaID, Name: "type"}, ID: 52},
+			},
+			expected: `Examining 2 descriptors and 2 namespace entries...
+  ParentID  51, ParentSchemaID 29: type "type" (52): validation: runtime error: invalid memory address or nil pointer dereference
 `,
 		},
 		{