avoid undefined behavior from UNREACHABLE on reachable code paths (scp-fs2open#7555)

* avoid undefined behavior from UNREACHABLE on reachable code paths

UNREACHABLE expands to __assume(false) / __builtin_unreachable() in
release builds, which is undefined behavior if the code is ever actually
reached. Many UNREACHABLE sites only guard against unexpected-but-
possible runtime states (values from tables, mission/save files, network
packets, scripting, or user input), so reaching them should degrade
gracefully rather than invoke UB.

Replace UNREACHABLE with Assertion(false, ...) at all such sites. It
still pops an error in debug builds, but in release it is a harmless
no-op that simply falls through to the following code. Sites that are
genuinely unreachable by local invariants (e.g. a switch over a bounded
random number) are left as UNREACHABLE.

A few converted sites had no safe fall-through -- an uninitialized read,
null dereference, or null call would have followed -- so they also get a
minimal recovery: initialize the value, guard the dereference, return
early, or skip the offending item.

Where the converted site was a simple guard of the form
    if (x) { Assertion(false, ...); return; }
hoist the check onto the invariant instead:
    Assertion(!x, ...);
    if (x) { return; }
This states the expectation directly and shows the offending value in
the debug popup, while keeping the same graceful runtime degradation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* addendum

After consultation with Lafiel, use UNREACHABLE in situations that are effectively impossible even if not provably impossible.  UNREACHABLE = "this shouldn't ever happen"; Assertion = "if this ever happens, it's a coding error".  Also fix a few other sites and change a couple to Warnings.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Goober5000

code/actions/expression/TypeDefinition.cpp

@@ -38,7 +38,7 @@ const TypeDefinition& TypeDefinition::forValueType(ValueType type)
 	case ValueType::String:
 		return s_identifier;
 	default:
-		UNREACHABLE("Invalid value type!");
+		UNREACHABLE("Invalid value type %d!", static_cast<int>(type));
 		return s_integer; // Make compiler happy
 	}
 }

code/ai/aibig.cpp

@@ -1870,7 +1870,7 @@ void ai_big_strafe()
 
 /*
 	if ( aip->goal_objnum != aip->target_objnum ) {
-		UNREACHABLE("The goal objnum does not match the target objnum in ai_big_strafe(), please report to the SCP!");	// what is going on here? - Get Alan
+		Assertion(false, "The goal objnum does not match the target objnum in ai_big_strafe(), please report to the SCP!");	// what is going on here? - Get Alan
 		aip->mode = AIM_NONE;
 		return;
 	}

code/ai/aicode.cpp

@@ -2734,8 +2734,8 @@ void ai_evade_object(object *evader, object *evaded)
 	Assert(evader->instance != -1);
 	Assert(Ships[evader->instance].ai_index != -1);
 
+	Assertion(evaded != evader, "Ship %s is trying to evade itself. Please report to the SCP!", Ships[evader->instance].ship_name);	//	Bogus!  Who tried to get me to evade myself!  Trace out and fix!
 	if (evaded == evader) {
-		UNREACHABLE("Ship %s is trying to evade itself. Please report to the SCP!", Ships[evader->instance].ship_name);	//	Bogus!  Who tried to get me to evade myself!  Trace out and fix!
 		return;
 	}
 
@@ -4997,8 +4997,8 @@ void ai_waypoints()
 	ai_info	*aip = &Ai_info[Ships[Pl_objp->instance].ai_index];
 
 	// sanity checking for stuff that should never happen
+	Assertion(aip->wp_index != INVALID_WAYPOINT_POSITION, "Waypoints should have been started already!");
 	if (aip->wp_index == INVALID_WAYPOINT_POSITION) {
-		UNREACHABLE("Waypoints should have been started already!");
 		ai_start_waypoints(Pl_objp, (aip->wp_list_index < 0) ? 0 : aip->wp_list_index, WPF_REPEAT, 0);
 	}
 
@@ -11020,8 +11020,8 @@ void ai_guard()
 
 	guard_objp = &Objects[aip->guard_objnum];
 
+	Assertion(guard_objp != Pl_objp, "The ship %s has been found to be guarding itself in ai_guard.  Please report to the SCP!", shipp->ship_name);		//	This seems illegal.  Why is a ship guarding itself?
 	if (guard_objp == Pl_objp) {
-		UNREACHABLE("The ship %s has been found to be guarding itself in ai_guard.  Please report to the SCP!", shipp->ship_name);		//	This seems illegal.  Why is a ship guarding itself?
 		aip->guard_objnum = -1;
 		return;
 	}
@@ -13889,7 +13889,7 @@ int ai_acquire_emerge_path(object *pl_objp, int parent_objnum, int allowed_path_
 		// Cyborg: to avoid divide by zero and avoid a logic path that should not exist check for 0 allowed paths -- Coverity 1523287
 		if (num_allowed_paths == 0) {
 			bay_path = Ai_last_arrive_path % bay->num_paths;
-			UNREACHABLE("Parent_shipp in ai_acquire_emerge_path somehow does not have any allowed bay paths!");
+			Assertion(false, "Parent_shipp in ai_acquire_emerge_path somehow does not have any allowed bay paths!");
 		} else {
 			// cycle through the allowed paths
 			bay_path = allowed_bay_paths[Ai_last_arrive_path % num_allowed_paths];
@@ -14382,7 +14382,7 @@ void ai_execute_behavior(ai_info *aip)
 		ai_lua(aip);
 		break;
 	default:
-		UNREACHABLE("Unknown AI Mode! Get a coder!");
+		UNREACHABLE("Unknown AI Mode %d! Get a coder!", aip->mode);
 		break;
 	}
 
@@ -15022,7 +15022,7 @@ int aas_1(object *objp, ai_info *aip, vec3d *safe_pos)
 		return 1;
 
 	} else {
-		UNREACHABLE("aas_1 has been passed an invalid object type of %d, please report to the SCP!", objp->type);
+		Assertion(false, "aas_1 has been passed an invalid object type of %d, please report to the SCP!", objp->type);
 	}
 
 	return 0;
@@ -16398,7 +16398,7 @@ void ai_ship_hit(object *objp_ship, object *hit_objp, const vec3d *hit_normal)
 			} else if (hit_objp->type == OBJ_SHIP) {
 				objp_hitter = hit_objp;
 			} else {
-				UNREACHABLE("Should never happen.");
+				Assertion(false, "ai_ship_hit has been passed an invalid object type of %d, please report to the SCP!", hit_objp->type);
 				return;
 			}
 			Assert(objp_hitter != nullptr);
@@ -16485,7 +16485,7 @@ void ai_ship_hit(object *objp_ship, object *hit_objp, const vec3d *hit_normal)
 		objp_hitter = hit_objp;
 		hitter_objnum = OBJ_INDEX(hit_objp);
 	} else {
-		UNREACHABLE("ai_ship_hit has been passed an invalid object type of %d, please report to the SCP!", hit_objp->type);
+		Assertion(false, "ai_ship_hit has been passed an invalid object type of %d, please report to the SCP!", hit_objp->type);
 		return;
 	}
 
@@ -16616,7 +16616,7 @@ void ai_ship_hit(object *objp_ship, object *hit_objp, const vec3d *hit_normal)
 	case AIM_LUA:
 		return;
 	default:
-		UNREACHABLE("Unknown AI Mode! Get a coder!");
+		UNREACHABLE("Unknown AI Mode %d! Get a coder!", aip->mode);
 	}
 
 	if (timestamp_elapsed(aip->ok_to_target_timestamp)) {

code/ai/aigoals.cpp

@@ -1253,7 +1253,7 @@ void ai_add_goal_sub_sexp( int sexp, ai_goal_type type, ai_info *aip, ai_goal *a
 		} else if ( op == OP_AI_IGNORE_NEW ) {
 			aigp->ai_mode = AI_GOAL_IGNORE_NEW;
 		} else
-			UNREACHABLE("Coding error: unhandled AI goal in ai_add_goal_sub_sexp!");
+			UNREACHABLE("Coding error: unhandled AI goal %d in ai_add_goal_sub_sexp!", op);
 
 		break;
 
@@ -2056,7 +2056,7 @@ ai_achievability ai_mission_goal_achievable( int objnum, ai_goal *aigp )
 			if (target_ship_entry->shipp()->subsys_info[SUBSYSTEM_TURRET].type_count == 0)
 				return ai_achievability::NOT_ACHIEVABLE;
 		} else {
-			UNREACHABLE("Target name %s is not an arrived ship!", aigp->target_name);
+			Assertion(false, "Target name %s is not an arrived ship!", aigp->target_name);
 			return ai_achievability::NOT_ACHIEVABLE;			// force this goal to be invalid
 		}
 	}
@@ -2105,8 +2105,8 @@ ai_achievability ai_mission_goal_achievable( int objnum, ai_goal *aigp )
 			return ai_achievability::NOT_KNOWN;
 
 		// we must also determine if we're prevented from docking for any reason
+		Assertion(target_ship_entry && target_ship_entry->has_shipp(), "Target name %s is not an arrived ship!", aigp->target_name);
 		if (!target_ship_entry || !target_ship_entry->has_shipp()) {
-			UNREACHABLE("Target name %s is not an arrived ship!", aigp->target_name);
 			return ai_achievability::NOT_ACHIEVABLE;			// force this goal to be invalid
 		}
 		auto goal_objp = target_ship_entry->objp();
@@ -2163,7 +2163,7 @@ ai_achievability ai_mission_goal_achievable( int objnum, ai_goal *aigp )
 				if (aip->goal_objnum != target_ship_entry->objnum)
 					return ai_achievability::NOT_KNOWN;
 			} else {
-				UNREACHABLE("Target name %s is not an arrived ship!", aigp->target_name);
+				Assertion(false, "Target name %s is not an arrived ship!", aigp->target_name);
 				return ai_achievability::NOT_ACHIEVABLE;			// force this goal to be invalid
 			}
 		}
@@ -2177,14 +2177,14 @@ ai_achievability ai_mission_goal_achievable( int objnum, ai_goal *aigp )
 				aigp->ai_submode = ship_find_subsys( target_ship_entry->shipp(), aigp->docker.name );
 				aigp->flags.remove(AI::Goal_Flags::Subsys_needs_fixup);
 			} else {
-				UNREACHABLE("Target name %s is not an arrived ship!", aigp->target_name);
+				Assertion(false, "Target name %s is not an arrived ship!", aigp->target_name);
 				return ai_achievability::NOT_ACHIEVABLE;			// force this goal to be invalid
 			}
 		}
 	} else if ( ((aigp->ai_mode == AI_GOAL_IGNORE) || (aigp->ai_mode == AI_GOAL_IGNORE_NEW)) && (status == SHIP_STATUS_ARRIVED) ) {
 		// for ignoring a ship, call the ai_ignore object function, then declare the goal satisfied
+		Assertion(target_ship_entry && target_ship_entry->has_objp(), "Target name %s is not an arrived ship!", aigp->target_name);
 		if (!target_ship_entry || !target_ship_entry->has_objp()) {
-			UNREACHABLE("Target name %s is not an arrived ship!", aigp->target_name);
 			return ai_achievability::NOT_ACHIEVABLE;			// force this goal to be invalid
 		}
 		auto ignored = target_ship_entry->objp();
@@ -2233,16 +2233,16 @@ ai_achievability ai_mission_goal_achievable( int objnum, ai_goal *aigp )
 		{
 			// short circuit a couple of cases.  Ship not arrived shouldn't happen.  Ship gone means
 			// we mark the goal as not achievable.
+			Assertion(status != SHIP_STATUS_NOT_ARRIVED, "Ship %s cannot rearm a target %s that hasn't arrived; get Allender or a SCP member", shipp->ship_name, aigp->target_name);	// get Allender.  this shouldn't happen!!!
 			if ( status == SHIP_STATUS_NOT_ARRIVED ) {
-				UNREACHABLE("Ship %s cannot rearm a target %s that hasn't arrived; get Allender or a SCP member", shipp->ship_name, aigp->target_name);	// get Allender.  this shouldn't happen!!!
 				return ai_achievability::NOT_ACHIEVABLE;
 			}
 
 			if ( status == SHIP_STATUS_GONE )
 				return ai_achievability::NOT_ACHIEVABLE;
 
+			Assertion(target_ship_entry && target_ship_entry->has_shipp(), "Target name %s is not an arrived ship!", aigp->target_name);
 			if ( !target_ship_entry || !target_ship_entry->has_shipp() ) {
-				UNREACHABLE("Target name %s is not an arrived ship!", aigp->target_name);
 				return ai_achievability::NOT_ACHIEVABLE;
 			}
 

code/bmpman/bmpman.cpp

@@ -599,8 +599,8 @@ int bm_create_3d(int bpp, int w, int h, int d, void* data) {
 		return -1;
 
 	// make sure that we have valid data
+	Assertion(data != nullptr, "No valid data received for 3D Bitmap creation!");
 	if (data == nullptr) {
-		UNREACHABLE("No valid data received for 3D Bitmap creation!");
 		return -1;
 	}
 
@@ -1210,7 +1210,7 @@ static int bm_load_info(BM_TYPE type, const char *filename, CFILE *img_cfp, int
 		}
 	}
 	else {
-		UNREACHABLE("Unknown file type specified! This is probably a coding error.");
+		UNREACHABLE("Unknown file type %d specified! This is probably a coding error.", type);
 
 		return -1;
 	}

code/cfile/cfile.cpp

@@ -832,7 +832,7 @@ static int cfget_cfile_block()
 	mprintf(("Out of cfile blocks! Currently opened files:\n"));
 	dump_opened_files();
 
-	UNREACHABLE("There are no more free cfile blocks. This means that there are too many files opened by FSO.\n"
+	Warning(LOCATION, "There are no more free cfile blocks. This means that there are too many files opened by FSO.\n"
 		"This is probably caused by a programming or scripting error where a file does not get closed."); // out of free cfile blocks
 	return -1;			
 }

code/controlconfig/controlsconfig.cpp

@@ -804,7 +804,7 @@ bool control_config_remove_binding(int ctrl, selItem item, bool API_Access)
 
 	default:
 		// Coder forgot to add a case!
-		UNREACHABLE("Unhandled selItem case.");
+		UNREACHABLE("Unhandled selItem case: %i", static_cast<int>(item));
 	}
 
 	if (success)
@@ -3120,7 +3120,7 @@ void control_get_axes_readings(int *axis_v, float frame_time)
 			case CC_TYPE_AXIS_BTN_POS:
 			default:
 				//This should never happen, especially with the above Assertion. This is required as incomplete switches on an enum generate warnings
-				UNREACHABLE("Unhandled control item type");
+				UNREACHABLE("Unhandled control item type %d", static_cast<int>(item.type));
 				break;
 			}
 		}

code/controlconfig/controlsconfigcommon.cpp

@@ -2388,7 +2388,7 @@ SCP_string ValToCID(CID id) {
 
 	if (it == mCIDNameToVal.cend()) {
 		// Shouldn't happen
-		Error(LOCATION, "Unknown CID value %i", id);
+		UNREACHABLE("Unknown CID value %i", id);
 		return "NONE";
 
 	} else {
@@ -2518,9 +2518,11 @@ SCP_string ValToJoy(const CC_bind &bind) {
 		if (it == mAxisNameToVal.end()) {
 			// should never happen
 			UNREACHABLE("Unknown error occured during reverse lookup of joy input string.");
-		} // else print out value
-
-		str = it->first;
+			str = "NONE";
+		} else {
+			// print out value
+			str = it->first;
+		}
 
 /*	} else if (bind.flags & CCF_HAT) {
 		// TODO Still currently encoded as buttons

code/cutscene/VideoPresenter.cpp

@@ -49,7 +49,7 @@ VideoPresenter::VideoPresenter(const MovieProperties& props) : _properties(props
 		material_set_unlit(&_rgb_material, _planeTextureHandles[0], 1.0f, true, false);
 		break;
 	default:
-		UNREACHABLE("Unhandled enum value!");
+		UNREACHABLE("Unhandled enum value %d!", static_cast<int>(props.pixelFormat));
 		break;
 	}
 }
@@ -86,7 +86,7 @@ void VideoPresenter::uploadVideoFrame(const VideoFramePtr& frame) {
 			bpp = 32;
 			break;
 		default:
-			UNREACHABLE("Unhandled enum value!");
+			UNREACHABLE("Unhandled enum value %d!", static_cast<int>(_properties.pixelFormat));
 			break;
 		}
 
@@ -145,7 +145,7 @@ void VideoPresenter::displayFrame(float x1, float y1, float x2, float y2, float
 		gr_render_primitives(&_rgb_material, PRIM_TYPE_TRISTRIP, &layout, 0, 4, gr_immediate_buffer_handle, offset);
 		break;
 	default:
-		UNREACHABLE("Unhandled enum value!");
+		UNREACHABLE("Unhandled enum value %d!", static_cast<int>(_properties.pixelFormat));
 		break;
 	}
 }

code/debris/debris.cpp

@@ -337,7 +337,7 @@ void debris_process_post(object * obj, float frame_time)
 						break;
 
 					default:
-						UNREACHABLE("Unhandled case %d for electrical arc creation in debris_process_post()!", n);
+						Assertion(false, "Unhandled case %d for electrical arc creation in debris_process_post()!", n);
 					}
 				}
 			}