[PATCH] Avoid harmful fall throughs in core_target::xfer_partial.

[PATCH] Avoid harmful fall throughs in core_target::xfer_partial.

[John Baldwin]

The cases for TARGET_OBJECT_LIBRARIES and TARGET_OBJECT_LIBRARIES_AIX
can try to fetch different data objects (such as
TARGET_OBJECT_SIGNAL_INFO) if gdbarch methods for the requested data
aren't present.  Replace case fallthroughs with an explicit goto to
the default case.
---
 gdb/corelow.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 1579e6bc2b8..f7f2bd3f318 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -943,7 +943,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 		return TARGET_XFER_OK;
 	    }
 	}
-      /* FALL THROUGH */
+      goto fallthrough;
 
     case TARGET_OBJECT_LIBRARIES_AIX:
       if (m_core_gdbarch != nullptr
@@ -964,7 +964,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
 		return TARGET_XFER_OK;
 	    }
 	}
-      /* FALL THROUGH */
+      goto fallthrough;
 
     case TARGET_OBJECT_SIGNAL_INFO:
       if (readbuf)
@@ -988,6 +988,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
       return TARGET_XFER_E_IO;
 
     default:
+    fallthrough:
       return this->beneath ()->xfer_partial (object, annex, readbuf,
 					     writebuf, offset, len,
 					     xfered_len);
-- 
2.34.1

Re: [PATCH] Avoid harmful fall throughs in core_target::xfer_partial.

[Pedro Alves]

On 2022-03-16 19:36, John Baldwin wrote:
> The cases for TARGET_OBJECT_LIBRARIES and TARGET_OBJECT_LIBRARIES_AIX
> can try to fetch different data objects (such as
> TARGET_OBJECT_SIGNAL_INFO) if gdbarch methods for the requested data
> aren't present.  Replace case fallthroughs with an explicit goto to
> the default case.

This is OK.  I mean, I think it would be reasonable to instead return
TARGET_XFER_E_IO -- it's what the other cases do (notice even TARGET_OBJECT_SIGNAL_INFO
doesn't fallback to the beneath target if there's no gdbarch method installed), and the targets
beneath (file and dummy) won't be returning any library anyhow.  OTOH, maybe some day we will
teach GDB to read the list of libraries the program is linked with and load those before
the program starts, and it's conceivable that we would do that at the file_stratum layer.

Pedro Alves

> ---
>  gdb/corelow.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 1579e6bc2b8..f7f2bd3f318 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -943,7 +943,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
>  		return TARGET_XFER_OK;
>  	    }
>  	}
> -      /* FALL THROUGH */
> +      goto fallthrough;
>  
>      case TARGET_OBJECT_LIBRARIES_AIX:
>        if (m_core_gdbarch != nullptr
> @@ -964,7 +964,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
>  		return TARGET_XFER_OK;
>  	    }
>  	}
> -      /* FALL THROUGH */
> +      goto fallthrough;
>  
>      case TARGET_OBJECT_SIGNAL_INFO:
>        if (readbuf)
> @@ -988,6 +988,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
>        return TARGET_XFER_E_IO;
>  
>      default:
> +    fallthrough:
>        return this->beneath ()->xfer_partial (object, annex, readbuf,
>  					     writebuf, offset, len,
>  					     xfered_len);

Re: [PATCH] Avoid harmful fall throughs in core_target::xfer_partial.

[John Baldwin]

On 3/17/22 9:06 AM, Pedro Alves wrote:
> On 2022-03-16 19:36, John Baldwin wrote:
>> The cases for TARGET_OBJECT_LIBRARIES and TARGET_OBJECT_LIBRARIES_AIX
>> can try to fetch different data objects (such as
>> TARGET_OBJECT_SIGNAL_INFO) if gdbarch methods for the requested data
>> aren't present.  Replace case fallthroughs with an explicit goto to
>> the default case.
> 
> This is OK.  I mean, I think it would be reasonable to instead return
> TARGET_XFER_E_IO -- it's what the other cases do (notice even TARGET_OBJECT_SIGNAL_INFO
> doesn't fallback to the beneath target if there's no gdbarch method installed), and the targets
> beneath (file and dummy) won't be returning any library anyhow.  OTOH, maybe some day we will
> teach GDB to read the list of libraries the program is linked with and load those before
> the program starts, and it's conceivable that we would do that at the file_stratum layer.

I'm happy to do whichever.  I was trying to preserve the intent from when
TARGET_OBJECT_LIBRARIES was first added.  If there are no beneath targets that
handle these objects, then I think it probably is cleaner to just fail with
TARGET_XFER_E_IO for now and let explicit fall through logic be added in the
future when it is needed.

> Pedro Alves
> 
>> ---
>>   gdb/corelow.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/corelow.c b/gdb/corelow.c
>> index 1579e6bc2b8..f7f2bd3f318 100644
>> --- a/gdb/corelow.c
>> +++ b/gdb/corelow.c
>> @@ -943,7 +943,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
>>   		return TARGET_XFER_OK;
>>   	    }
>>   	}
>> -      /* FALL THROUGH */
>> +      goto fallthrough;
>>   
>>       case TARGET_OBJECT_LIBRARIES_AIX:
>>         if (m_core_gdbarch != nullptr
>> @@ -964,7 +964,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
>>   		return TARGET_XFER_OK;
>>   	    }
>>   	}
>> -      /* FALL THROUGH */
>> +      goto fallthrough;
>>   
>>       case TARGET_OBJECT_SIGNAL_INFO:
>>         if (readbuf)
>> @@ -988,6 +988,7 @@ core_target::xfer_partial (enum target_object object, const char *annex,
>>         return TARGET_XFER_E_IO;
>>   
>>       default:
>> +    fallthrough:
>>         return this->beneath ()->xfer_partial (object, annex, readbuf,
>>   					     writebuf, offset, len,
>>   					     xfered_len);
> 


-- 
John Baldwin

Re: [PATCH] Avoid harmful fall throughs in core_target::xfer_partial.

[Pedro Alves]

On 2022-03-17 16:23, John Baldwin wrote:
> On 3/17/22 9:06 AM, Pedro Alves wrote:
>> On 2022-03-16 19:36, John Baldwin wrote:
>>> The cases for TARGET_OBJECT_LIBRARIES and TARGET_OBJECT_LIBRARIES_AIX
>>> can try to fetch different data objects (such as
>>> TARGET_OBJECT_SIGNAL_INFO) if gdbarch methods for the requested data
>>> aren't present.  Replace case fallthroughs with an explicit goto to
>>> the default case.
>>
>> This is OK.  I mean, I think it would be reasonable to instead return
>> TARGET_XFER_E_IO -- it's what the other cases do (notice even TARGET_OBJECT_SIGNAL_INFO
>> doesn't fallback to the beneath target if there's no gdbarch method installed), and the targets
>> beneath (file and dummy) won't be returning any library anyhow.  OTOH, maybe some day we will
>> teach GDB to read the list of libraries the program is linked with and load those before
>> the program starts, and it's conceivable that we would do that at the file_stratum layer.
> 
> I'm happy to do whichever.  I was trying to preserve the intent from when
> TARGET_OBJECT_LIBRARIES was first added.  If there are no beneath targets that
> handle these objects, then I think it probably is cleaner to just fail with
> TARGET_XFER_E_IO for now and let explicit fall through logic be added in the
> future when it is needed.

Fine with me.  Consider it pre-approved.

Thanks.